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)
Tracking
()
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: firefox, Assigned: bzbarsky)
References
Details
(5 keywords)
Attachments
(4 files, 1 obsolete file)
(deleted),
application/java-archive
|
Details | |
(deleted),
patch
|
enndeakin
:
review+
roc
:
superreview+
beltzner
:
approval1.9.1+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
samuel.sidler+old
:
approval1.9.0.11+
samuel.sidler+old
:
approval1.9.0.12+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
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.
Comment 2•15 years ago
|
||
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
Comment 3•15 years ago
|
||
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?
Assignee | ||
Comment 4•15 years ago
|
||
Verified, via local backout, that this is a regression from bug 484031. Looking into it.
Assignee: nobody → bzbarsky
Blocks: 484031
Assignee | ||
Comment 5•15 years ago
|
||
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.
Assignee | ||
Comment 6•15 years ago
|
||
Attachment #380769 -
Flags: superreview?(roc)
Attachment #380769 -
Flags: review?(roc)
Comment 7•15 years ago
|
||
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?
Assignee | ||
Comment 8•15 years ago
|
||
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...
Comment 9•15 years ago
|
||
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-
Updated•15 years ago
|
Attachment #380769 -
Flags: review?(roc) → review+
Assignee | ||
Comment 10•15 years ago
|
||
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•15 years ago
|
Attachment #380769 -
Flags: approval1.9.1?
Assignee | ||
Comment 11•15 years ago
|
||
Comment on attachment 380769 [details] [diff] [review]
Proposed fix
This is looking green on trunk.
Comment 12•15 years ago
|
||
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+
Assignee | ||
Comment 13•15 years ago
|
||
Flags: in-testsuite+
Keywords: fixed1.9.1
Assignee | ||
Comment 14•15 years ago
|
||
Attachment #380908 -
Flags: approval1.9.0.12?
Updated•15 years ago
|
Attachment #380652 -
Attachment mime type: application/x-file-download → application/java-archive
Assignee | ||
Comment 15•15 years ago
|
||
Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/2f33f928640f to fix resulting 1.9.1 crashes.
Assignee | ||
Comment 16•15 years ago
|
||
Attachment #380908 -
Attachment is obsolete: true
Attachment #380934 -
Flags: approval1.9.0.12?
Attachment #380908 -
Flags: approval1.9.0.12?
Comment 17•15 years ago
|
||
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 ...
Attachment #380769 -
Flags: superreview?(roc) → superreview+
Assignee | ||
Comment 18•15 years ago
|
||
This issue is listbox-specific.
Updated•15 years ago
|
Target Milestone: --- → mozilla1.9.2a1
Comment 19•15 years ago
|
||
Maybe the Music Player Minion playlist then, or yoono https://addons.mozilla.org/en-US/firefox/addon/1833
Comment 20•15 years ago
|
||
Can't repro the bug with yoono. The variable name/element ID in the script said "listbox" but it's really a <tree>
Comment 21•15 years ago
|
||
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
Comment 22•15 years ago
|
||
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 23•15 years ago
|
||
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+
Comment 24•15 years ago
|
||
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
Comment 25•15 years ago
|
||
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.
Updated•15 years ago
|
Keywords: fixed1.9.0.11 → verified1.9.0.11
Comment 26•15 years ago
|
||
Updated•15 years ago
|
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.next+
Comment 27•10 years ago
|
||
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.
Description
•