Opened 7 years ago

Closed 7 years ago

#294 closed defect (fixed)

setFeature is called with the previous geometry

Reported by: bartvde Owned by:
Priority: major Milestone: 1.0
Component: GeoExt.plugins.PrintExtent Version: trunk
Keywords: Cc:
State: Commit

Description

When an application has multiple layouts with different sizes or orientation, changing the layout will get the transform feature vectors disconnected from the orange box geometry. This is caused by a missing call to fitPage.

Attachments (3)

ticket294.patch (715 bytes) - added by bartvde 7 years ago.
possible solution
294.tar (20.0 KB) - added by bartvde 7 years ago.
example based on simpleprint ux to reproduce
geoext-294.patch (1.3 KB) - added by ahocevar 7 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 follow-up: Changed 7 years ago by bartvde

The problem seems to be that PrintExtent::updateBox is called before PrintPage::updateFeature finishes. And since only the geometry has changed, no page change event is triggered (modified=false).

comment:2 Changed 7 years ago by bartvde

Not sure what is the right way to fix this, e.g. we can also trigger the page change event if the geometry changes. Another idea Andreas?

Changed 7 years ago by bartvde

possible solution

comment:3 in reply to: ↑ 1 Changed 7 years ago by ahocevar

Replying to bartvde:

The problem seems to be that PrintExtent::updateBox is called before PrintPage::updateFeature finishes. And since only the geometry has changed, no page change event is triggered (modified=false).

Since there is no setTimeout magic going on, everything runs in one thread, and the reason cannot be updateBox being called before updateFeature finishes. Bart, can you please provide a minimal testcase so we can investigate what is going on here?

comment:4 Changed 7 years ago by bartvde

I'll try to do that Andreas, I had some trouble before isolating the problem.

Changed 7 years ago by bartvde

example based on simpleprint ux to reproduce

comment:5 Changed 7 years ago by bartvde

Andreas, I've an example to reproduce. If you switch layout from no. 1 to no. 2 you'll see the issue.

Changed 7 years ago by ahocevar

comment:6 Changed 7 years ago by ahocevar

  • State changed from None to Review

attachment:geoext-294.patch fixes the evil at its root: the PrintExtent does not need to know about a layout change, only the page needs to handle it correctly.

Tests continue to pass. Please review.

comment:7 Changed 7 years ago by bartvde

Thanks Andreas, this looks good, I'll be able to check this next Monday and I'll report back then.

comment:8 Changed 7 years ago by bartvde

  • State changed from Review to Commit

Thanks for the updated patch Andreas, looks good and works for me, please commit.

comment:9 Changed 7 years ago by ahocevar

  • Resolution set to fixed
  • Status changed from new to closed

(In [2237]) Make PrintPage handle layout changes correctly so PrintExtent does not need to know about layout changes any more. r=bartvde (closes #294)

Note: See TracTickets for help on using tickets.