Ticket #106 (closed defect: fixed)

Opened 9 months ago

Last modified 8 months ago

layers set in reverse order

Reported by: fredj Assigned to:
Priority: critical Milestone: 0.5
Component: GeoExt.data.LayerStore Version: trunk
Keywords: Cc:
State: Complete

Description

The layers array added the a GeoExt.data.LayerStore are set the the map but in reverse order: if the layers list is [a, b, c], the map.layers will be [c, b, a].

Attachments

106.patch (1.6 kB) - added by fredj on 06/24/09 14:06:02.
A unit test to demonstrate the issue.
106.2.patch (2.3 kB) - added by fredj on 06/24/09 14:46:11.
don't set the layers in reverse order, with units test
106.3.patch (2.3 kB) - added by tschaub on 06/24/09 23:06:04.
patch against trunk/geoext

Change History

06/24/09 14:06:02 changed by fredj

  • attachment 106.patch added.

A unit test to demonstrate the issue.

06/24/09 14:11:22 changed by fredj

  • owner changed.
  • component changed from data to data.LayerStore.

06/24/09 14:46:11 changed by fredj

  • attachment 106.2.patch added.

don't set the layers in reverse order, with units test

06/24/09 14:48:00 changed by fredj

  • state changed from None to Review.

The patch set the layers from the store to the map in the correct order (not in the reverse order), all tests pass on FF 3.0.11.

Please review.

06/24/09 16:17:15 changed by fredj

(In [1154]) fix 'layers set in reverse order' (see #106)

06/24/09 23:06:04 changed by tschaub

  • attachment 106.3.patch added.

patch against trunk/geoext

06/24/09 23:07:36 changed by tschaub

  • version changed from SVN to trunk.

Thanks for catching (and fixing) this fredj. Sort of surprising it didn't come up earlier. The latest patch is the same as yours, but applies to trunk/geoext.

06/24/09 23:10:17 changed by tschaub

The fix looks good. I'm seeing an error in Map.js when running tests (likely unrelated). I'll check that out now, but I think this is good to go. Thanks for the nice tests.

06/24/09 23:24:27 changed by tschaub

  • state changed from Review to Commit.

Ok, the error comes in the first test of Popup.html and is unrelated to this change. Please commit (and set ticket state to "Pullup").

06/25/09 08:10:22 changed by fredj

  • state changed from Commit to Pullup.

06/25/09 19:13:56 changed by tschaub

Thanks for getting this in. Please include a revision number for the change before closing (or marking pullup).

06/25/09 19:15:05 changed by tschaub

Ah, I see that "pullup" doesn't mean anything to the post commit hook. This went in with r1156.

06/29/09 11:38:27 changed by ahocevar

  • status changed from new to closed.
  • resolution set to fixed.

(In [1159]) Ticket pullups since 0.5 RC1 (closes #103, #106, #107)

07/02/09 10:22:14 changed by ahocevar

  • state changed from Pullup to Complete.