Closed Bug 364762 Opened 14 years ago Closed 14 years ago

[FIX][reflow branch] list is truncated when slider is moved

Categories

(Core :: Layout: Form Controls, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: xanthian, Assigned: bzbarsky)

References

()

Details

(Whiteboard: URL is testcase, see comment #6)

Attachments

(2 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061217 SeaMonkey/1.5a
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061217 SeaMonkey/1.5a

This one is complicated to reproduce, but fails as indicated every time.
The issue is that the "choose a comic" menu, which has about 300 entries
(see frame URL , eg, (maybe)
http://www.thedreamlandchronicles.com/cgi-bin/autokeenlite.cgi?date=20061222
for menu source HTML code) works okay when it is opened for the current day
and the slider is used. However, navigate to the previous day, move the
slider even a tiny amount, and suddenly only the first 141 menu entries are
accessible.

Reproducible: Always

Steps to Reproduce:
1. Open URL
2. Check that the comic day selection menu works okay
3. click "previous" button to go to prior day's comic
4. check that menu now truncates as soon as the vertical slider is moved at all

Actual Results:  
Menu is inappropriately truncated as soon as the slider is moved, for days
other than the current day (and yet, frame HTML source code does contain
the whole menu).

Expected Results:  
Menu works the same when used from any day.

There are several workarounds, but to a naive user, this is a bit of a mess.
Confirming with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/2006122214 SeaMonkey/1.5a (self compiled suiterunner build).

When I move the slider, the menu is truncated and some entries (for me it were all > 103) aren't accessible with the mouse. But they can still be accessed with the cursor keys.

I can't reproduce the problem with SM 1.1 and I also have a WFM with a trunk build from 20061118.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Product: Mozilla Application Suite → Core
Version: unspecified → Trunk
Component: General → Layout: Form Controls
Assignee: general → nobody
QA Contact: general → layout.form-controls
WFM, Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9a2pre) Gecko/20061222 SeaMonkey/1.5a
OS: All → Windows XP
I can confirm this on Mac OS X for a custom debug trunk build [Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a2pre) Gecko/20061219 SeaMonkey/1.5a]. 
I don't even have to go to other pages, the given URL does suffice.
When I open the dropdownbox, the current entry is selected, but the scrollbar thumb already has the wrong size. Clicking the down arrow restricts the menulist to the length of the scrollbar thumb. But this behaviour is not always reproducable, sometimes reloading the page makes the problem go away. And I can't find a specific reason for the actual cut-point - I had 131, 171, 224, ... 
OS: Windows XP → All
Hardware: PC → All
Works in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/2006120704 Minefield/3.0a1
Fails in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/2006120804 Minefield/3.0a1
Also fails in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/2006122904 Minefield/3.0a2pre

So probably another regression from reflow branch landing. I'll try to make a testcase for this.

Summary: SeaMonkey nightly: comic page long internal menu works for current day, fails from prior days as menu is truncated when slider is moved → [reflow-branch] menu is truncated when slider is moved
I found out another thing, the pages from http://www.thedreamlandchronicles.com/ work fine, when they are saved to the harddrive and opened from there. Uploading the saved page to a webserver shows the error again.
I created a testcase at http://www.aquachan.de/moz/bug364762/testcase3.php

With bigger numbers in the "Number of elements" field, the list shown at the top is cut down. I can't say exactly when this is going to happen, e.g. 200 works fine here, as does 1100, but 500 fails. I've also seen cases, where a certain number works fine, but fails at another load of the page and vice versa.

Same strange things seem to happen here.
Summary: [reflow-branch] menu is truncated when slider is moved → [reflow-branch] list is truncated when slider is moved
Whiteboard: URL is testcase, see comment #6
It'd be great to have a testcase in bugzilla here...

I'll try to look into this on Sunday, I guess.
Attached file static html testcase with 500 elements (obsolete) —
This is a static testcase with 500 <option> elements.

Steps to reproduce:

1. open testcase
2. move the slider with the mouse to the bottom of the <select> list

When the slider is at the bottom, it should show 500 but does show something else (196 at the moment). The other elements are still reachable with the cursor keys.

I can only see the problem, when the testcase is loaded from a webserver. When it's loaded from harddrive or it's cached somehow, I can't see the problem.

I'm not sure, if this testcase works for everyone and every connection. I tried to make a dynamic testcase with javascript, but I can't see the problem, probably cause the <select> list is created after the page is loaded.
(In reply to comment #8)
> Created an attachment (id=250488) [details]
The testcase isn't very reliable, I only see the problem in about one of five cases.
I testet it and it was fixed by bug 365837.

-> resolving as dupe
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 365837
Well, it seems indeed fixed now, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070114 Minefield/3.0a2pre
I could reproduce the bug with the testcase at http://www.aquachan.de/moz/bug364762/testcase3.php , but not anymore.
It seems odd to me though, that the patch for bug 365837 fixed this.
(In reply to comment #12)
> It seems odd to me though, that the patch for bug 365837 fixed this.
Maybe bz can explain, why this fixed it. I can only say, that without the fix I see the error and after applying I can't see it anymore with my testcase and also with the url from the bug reporter.
I'm going to reopen this.  While it's sorta gone with the patch for bug 365837, for reasons that I don't care enough to figure out, the underlying problem is still there.

The issue here is that we're doing an incremental reflow of the select.  So we go  and try to figure out whether we can get away with a one-pass reflow.  We can't, since the height of the scrolled frame changes.  So we do a second pass.

Since we're doing two passes, we suppress scrollbar updates on the first pass.

Now the thing is, our algorithm for deciding whether we can get away with a single pass is conservative -- we sometimes do two passes when we don't really need to.  Bug 314939 has some discussion of that.

This is one of those times.  So we reflow the scrollframe once with scrollbar updates suppressed, then again without.  But during the second reflow, all of:

  reflowHScrollbar, reflowVScrollbar, reflowScrollCorner, 
  (GetStateBits() & NS_FRAME_IS_DIRTY) != 0,
  didHaveHScrollbar != state.mShowHScrollbar,
  didHaveVScrollbar != state.mShowVScrollbar,
  oldScrollAreaBounds != newScrollAreaBounds,
  oldScrolledAreaBounds != newScrolledAreaBounds

are false.  So we never do the scrollbar update.

As I see it the options are to either make the conditions in nsSelectsAreaFrame exact (that is, reflow only when we really do need to reflow, such that during the second pass one of the tests above will be true) or to do something in scrollframe where if we bailed out because scrollbar updates were disabled the next time through we skip the above check.

roc, thoughts?
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
I checked, and for some reason in current trunk reflowHScrollbar, reflowVScrollbar, reflowScrollCorner are always true.  This happens because ShouldReflowAllKids() is always true, which happens because the state has mFlags.mHResize set.

And that happens because we're messing with the computed width but the reflow state has no way to know that, of course.
Attachment #250488 - Attachment is obsolete: true
I favour the latter:
> do something in
> scrollframe where if we bailed out because scrollbar updates were disabled the
> next time through we skip the above check.

Probably when mSuppressScrollbarUpdate is set, and we would have changed the scrollbar state, we should set a new flag mNeedScrollbarUpdate. Then when we check the next time whether we need to update the scrollbars, we should always do it if mNeedScrollbarUpdate is true (and then clear that flag) even if the state hasn't changed in this reflow.

In other words SetSuppressScrollbarUpdate merely defers scrollbar updates to the next reflow instead of dropping them on the floor.
Attached patch fix (obsolete) — Splinter Review
Yeah, that's what I'd sort of decided too.
Assignee: nobody → bzbarsky
Status: REOPENED → ASSIGNED
Attachment #252000 - Flags: superreview?(roc)
Attachment #252000 - Flags: review?(roc)
Summary: [reflow-branch] list is truncated when slider is moved → [FIX][reflow branch] list is truncated when slider is moved
Target Milestone: --- → mozilla1.9alpha
Why don't you make the big "has the state changed" check go on the outside, so we only set mSkippedScrollbarLayout if the state changed and mSkippedScrollbarLayout was set?
That would mean updating mInner.mHasHorizontalScrollbar and mInner.mHasVerticalScrollbar even if mSkippedScrollbarLayout is set.  Is that ok?
Attached patch With that changeSplinter Review
Attachment #252000 - Attachment is obsolete: true
Attachment #252009 - Flags: superreview?(roc)
Attachment #252009 - Flags: review?(roc)
Attachment #252000 - Flags: superreview?(roc)
Attachment #252000 - Flags: review?(roc)
Attachment #252009 - Flags: superreview?(roc)
Attachment #252009 - Flags: superreview+
Attachment #252009 - Flags: review?(roc)
Attachment #252009 - Flags: review+
Fixed.  Not sure how to test this....
Status: ASSIGNED → RESOLVED
Closed: 14 years ago14 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Flags: in-testsuite- → in-testsuite?
(In reply to comment #22)
> Fixed.  Not sure how to test this....

Just like to confirm WFM on the original
reported URL as of 2007012009 build, if
that helps. Thanks for the fast repair,
Boris.

xanthian.


You need to log in before you can comment on or make changes to this bug.