Closed Bug 495648 Opened 13 years ago Closed 13 years ago

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


(Core :: XUL, defect)

Windows Vista
Not set





(Reporter: firefox, Assigned: bzbarsky)



(5 keywords)


(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 (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.
Confirmed with latest trunk on Windows XP and Vista

Regression range:
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.
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 fixSplinter 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, 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+
Closed: 13 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) — Splinter Review
Attachment #380908 - Flags: approval1.9.0.12?
Attachment #380652 - Attachment mime type: application/x-file-download → application/java-archive
Pushed to fix resulting 1.9.1 crashes.
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)
 SiteDelta (sidebar)
 FoxyTunes (maybe? in the included jslib, not sure they use it)
 Music Player Minion
 Tab Mix (not TMPlus)
 Delicious Complete
  ... 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
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.

 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

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 and 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
Checking in tests/;
/cvsroot/mozilla/layout/base/tests/,v  <--
new revision: 1.64; previous revision: 1.63
RCS file: /cvsroot/mozilla/layout/base/tests/bug495648.rdf,v
Checking in tests/bug495648.rdf;
/cvsroot/mozilla/layout/base/tests/bug495648.rdf,v  <--  bug495648.rdf
initial revision: 1.1
RCS file: /cvsroot/mozilla/layout/base/tests/test_bug495648.xul,v
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
Checking in tests/;
/cvsroot/mozilla/layout/base/tests/,v  <--
new revision:; previous revision: 1.63
Checking in tests/bug495648.rdf;
/cvsroot/mozilla/layout/base/tests/bug495648.rdf,v  <--  bug495648.rdf
new revision:; previous revision:
Checking in tests/test_bug495648.xul;
/cvsroot/mozilla/layout/base/tests/test_bug495648.xul,v  <--  test_bug495648.xul
new revision:; previous revision:
Verified for with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv: Gecko/2009060214 Firefox/3.0.11.
Flags: wanted1.8.1.x+
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.