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

RESOLVED FIXED in mozilla1.9.2a1

Status

()

RESOLVED FIXED
9 years ago
4 years ago

People

(Reporter: firefox, Assigned: bzbarsky)

Tracking

(5 keywords)

Trunk
mozilla1.9.2a1
x86
Windows Vista
fixed1.9.0.12, fixed1.9.1, regression, testcase, verified1.9.0.11
Points:
---
Bug Flags:
blocking1.9.1 -
wanted1.9.1.x +
blocking1.9.0.11 +
blocking1.9.0.12 +
blocking1.8.1.next +
wanted1.8.1.x +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

9 years ago
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.
(Reporter)

Comment 1

9 years ago
Created attachment 380652 [details]
Testcase (requires chrome privileges)
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.
Created attachment 380769 [details] [diff] [review]
Proposed fix
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+
Pushed http://hg.mozilla.org/mozilla-central/rev/fe91973cc783
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
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+
Attachment #380652 - Attachment mime type: application/x-file-download → application/java-archive
Created attachment 380934 [details] [diff] [review]
Updated 1.9.0 merge
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
Keywords: fixed1.9.0.11, fixed1.9.0.12
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.
Keywords: fixed1.9.0.11 → verified1.9.0.11
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.