Closed Bug 495648 Opened 15 years ago Closed 15 years ago

Incorrect itemCount and scrolling of listboxes with XUL templates for RDF datasources

Categories

(Core :: XUL, defect)

x86
Windows Vista
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: firefox, Assigned: bzbarsky)

References

Details

(5 keywords)

Attachments

(4 files, 1 obsolete file)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; de-DE; rv:1.9.1b4) Gecko/20090423 Firefox/3.5b4 (.NET CLR 3.5.30729) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; de-DE; rv:1.9.1b4) Gecko/20090423 Firefox/3.5b4 (.NET CLR 3.5.30729) When dynamically adding a RDF datasource to a listbox and rebuilding the XUL templates, the listbox shows the right entries, but itemCount is always 1. When the listbox has scrollbars, those are displayed incorrectly (claiming there is only one item below), and scrolling always jumps back to the top. Reproducible: Always Steps to Reproduce: 1. Get the testcase from http://schierla.de/tmp/testcase/testcase.zip (requires chrome privileges) 2. open testcase.xul 3. click the "count" button below 4. try to scroll down the list (if scrollbars are present) Actual Results: The count button shows "1" as an item count, though 40 items are shown. Scrolling down does not work. Expected Results: The count button shows "40". Scrolling down the list works. This behaviour is new in FF 3.5b4, it was not yet present in 3.1b3. This behaviour occurs 3.0.12pre, but does not occur in 3.0.10.
Attached file Testcase (requires chrome privileges) (deleted) —
Confirmed with latest trunk on Windows XP and Vista Regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=54c37f642c5c&tochange=615c57912892 I don't know why, but I get not always 1 but also 3 as a result. On Windows XP always 1, and on Vista 3.
Status: UNCONFIRMED → NEW
Component: General → XUL
Ever confirmed: true
Keywords: regression, testcase
Product: Firefox → Core
QA Contact: general → xptoolkit.widgets
Version: unspecified → Trunk
None of the other bugs in the range are checked in into 1.9.0 so it must be bug 484031.
Flags: blocking1.9.1?
Flags: blocking1.9.0.12?
Verified, via local backout, that this is a regression from bug 484031. Looking into it.
Assignee: nobody → bzbarsky
Blocks: 484031
OK, this is ridiculous... What happens is that notifying the listbox of the very first listitem calls CreateRows, which happily creates _all_ the listitem frames (well, all the ones that fit in the listbox). So we don't notify it on any more items, which makes it confused about its count. I think creating those frames for rows that it hasn't counted yet is competely daft, but then again all this code is completely daft. We really need to remove it. :( Unless someone has a bright idea, I will likely hack around this in the check added for bug 484031. It'll slow down things generated via the template builder, but I'm sort of failing to care at this point.
Attached patch Proposed fix (deleted) — Splinter Review
Attachment #380769 - Flags: superreview?(roc)
Attachment #380769 - Flags: review?(roc)
Boris: do you think this needs to block the release, especially since it's new functionality that nobody's likely depending on, or can we mop it up in 3.5.1?
It's not new functionality: see comment 0. It's a regression from a security fix we landed. We need to fix this on all branches where we introduced it, including m-c, 1.9.1, and 1.9.0. If review comes in fast, I'd prefer to fix this in 1.9.1.0, but I wouldn't necessarily hold the release for it (esp. since we've already broken this on 1.9.0 and need to fix it there too); this would certainly be something that can go in a security update...
I hate myself for not blocking on this. If & when review comes, this has my pre-approval to land on trunk for baking and should be nominated for 191 once it goes green there. Someone should also poke me or shaver directly to get the flag set for a191.
Flags: wanted1.9.1.x+
Flags: blocking1.9.1?
Flags: blocking1.9.1-
Attachment #380769 - Flags: review?(roc) → review+
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Attachment #380769 - Flags: approval1.9.1?
Comment on attachment 380769 [details] [diff] [review] Proposed fix This is looking green on trunk.
Comment on attachment 380769 [details] [diff] [review] Proposed fix a191=beltzner; roc, please do let us know if this looks wrong, but so far it's looking really really right!
Attachment #380769 - Flags: approval1.9.1? → approval1.9.1+
Attached patch 1.9.0 merge (obsolete) (deleted) — Splinter Review
Attachment #380908 - Flags: approval1.9.0.12?
Attachment #380652 - Attachment mime type: application/x-file-download → application/java-archive
Attached patch Updated 1.9.0 merge (deleted) — Splinter Review
Attachment #380908 - Attachment is obsolete: true
Attachment #380934 - Flags: approval1.9.0.12?
Attachment #380908 - Flags: approval1.9.0.12?
qawanted: This may be breaking addons, and therefore be a stop-ship re-spin 3.0.11 rather than ship a security update that makes people want to downgrade. A quick add-on grep for AddDataSource() gets hits in at least 150 different files (partial collection of addons). Some might be listboxes, some are definitely trees or menus -- do those have problems too? Here are a few to check out: DOM Inspector (addons 1806, 1837, 2334) Torbutton https://addons.mozilla.org/en-US/firefox/addon/2275 Subtile https://addons.mozilla.org/en-US/firefox/addon/4906 SiteDelta https://addons.mozilla.org/en-US/firefox/addon/4337 (sidebar) FoxyTunes (maybe? in the included jslib, not sure they use it) https://addons.mozilla.org/en-US/firefox/addon/219 Mnenhy https://addons.mozilla.org/en-US/firefox/addon/2516 Hermes https://addons.mozilla.org/en-US/firefox/addon/1460 Music Player Minion https://addons.mozilla.org/en-US/firefox/addon/6324 Tab Mix (not TMPlus) https://addons.mozilla.org/en-US/firefox/addon/625 Delicious Complete https://addons.mozilla.org/en-US/firefox/addon/2354 ... and more ...
Flags: blocking1.9.0.12?
Flags: blocking1.9.0.12+
Flags: blocking1.9.0.11+
Keywords: qawanted
Attachment #380769 - Flags: superreview?(roc) → superreview+
This issue is listbox-specific.
Target Milestone: --- → mozilla1.9.2a1
Maybe the Music Player Minion playlist then, or yoono https://addons.mozilla.org/en-US/firefox/addon/1833
Can't repro the bug with yoono. The variable name/element ID in the script said "listbox" but it's really a <tree>
The bug reporter's own SiteDelta exhibits the problem. Guess that one isn't in the dataset I was scanning, or it was an older version that didn't use AddDataSource. https://addons.mozilla.org/en-US/firefox/addon/4337 STR/verify: 1. Install SiteDelta 2. Surf around and click the delta in the statusbar to add sites to list 3. open sitedelta sidebar if not already open 4. shrink page so page list gets a scrollbar 5. try to scroll list
Another Add-on that shows the problem is Download Sort https://addons.mozilla.org/en-US/firefox/addon/25 Open the pref dialog (from the addon dialog "preferences" button, dunno if there's another way) and add more than 12 extension rules so that the slider appears on the listbox. In 3.0.11 it won't scroll. Download Sort has 10 times the daily users of SiteDelta, although the broken behavior would be less obvious than SiteDelta's to existing users since they have probably already set up their preferences. Between the two that's probably enough to justify a stop-ship on 3.0.11, and there are likely other addons affected.
Comment on attachment 380934 [details] [diff] [review] Updated 1.9.0 merge Alright, let's get this on 1.9.0 and on the GECKO190_20090519_RELBRANCH. Approved for 1.9.0.11 and 1.9.0.12. a=ss
Attachment #380934 - Attachment description: Updated 1.9.1 merge → Updated 1.9.0 merge
Attachment #380934 - Flags: approval1.9.0.12?
Attachment #380934 - Flags: approval1.9.0.12+
Attachment #380934 - Flags: approval1.9.0.11+
Checked into cvs trunk and relbranch Checking in nsCSSFrameConstructor.cpp; /cvsroot/mozilla/layout/base/nsCSSFrameConstructor.cpp,v <-- nsCSSFrameConstructor.cpp new revision: 1.1483; previous revision: 1.1482 done Checking in tests/Makefile.in; /cvsroot/mozilla/layout/base/tests/Makefile.in,v <-- Makefile.in new revision: 1.64; previous revision: 1.63 done RCS file: /cvsroot/mozilla/layout/base/tests/bug495648.rdf,v done Checking in tests/bug495648.rdf; /cvsroot/mozilla/layout/base/tests/bug495648.rdf,v <-- bug495648.rdf initial revision: 1.1 done RCS file: /cvsroot/mozilla/layout/base/tests/test_bug495648.xul,v done Checking in tests/test_bug495648.xul; /cvsroot/mozilla/layout/base/tests/test_bug495648.xul,v <-- test_bug495648.xul initial revision: 1.1 Checking in nsCSSFrameConstructor.cpp; /cvsroot/mozilla/layout/base/nsCSSFrameConstructor.cpp,v <-- nsCSSFrameConstructor.cpp new revision: 1.1482.2.1; previous revision: 1.1482 done Checking in tests/Makefile.in; /cvsroot/mozilla/layout/base/tests/Makefile.in,v <-- Makefile.in new revision: 1.63.24.1; previous revision: 1.63 done Checking in tests/bug495648.rdf; /cvsroot/mozilla/layout/base/tests/bug495648.rdf,v <-- bug495648.rdf new revision: 1.1.2.2; previous revision: 1.1.2.1 done Checking in tests/test_bug495648.xul; /cvsroot/mozilla/layout/base/tests/test_bug495648.xul,v <-- test_bug495648.xul new revision: 1.1.2.2; previous revision: 1.1.2.1 done
Verified for 1.9.0.11 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.11) Gecko/2009060214 Firefox/3.0.11.
Attached patch 1.8 patch (deleted) — Splinter Review
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.next+
Issue is Resolved - removing QA-Wanted Keywords - QA-Wanted query clean-up task
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: