Last Comment Bug 388583 - Autocomplete dropdown doesn't update when scrolling with scrollbar
: Autocomplete dropdown doesn't update when scrolling with scrollbar
Status: VERIFIED FIXED
: fixed-seamonkey1.1.3, fixed1.8.0.13, pp, regression, verified1.8.1.6, verified1.8.1.8
Product: Core
Classification: Components
Component: XUL (show other bugs)
: 1.8 Branch
: PowerPC Mac OS X
: -- normal (vote)
: ---
Assigned To: Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
:
Mentors:
: 388711 388734 388775 388798 389044 389047 389078 389079 389124 389166 389207 389335 389371 389657 389980 390005 (view as bug list)
Depends on: 389259
Blocks: 344228 382444
  Show dependency treegraph
 
Reported: 2007-07-18 09:07 PDT by Toh Yanxi
Modified: 2011-05-24 20:58 PDT (History)
34 users (show)
mconnor: blocking1.8.1.6+
dveditz: blocking1.8.1.8+
dveditz: blocking1.8.0.13+
mozillamarcia.knous: in‑litmus-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
a guess fix. (2.50 KB, patch)
2007-07-18 15:09 PDT, Olli Pettay [:smaug]
no flags Details | Diff | Review
fix (3.14 KB, patch)
2007-07-18 16:05 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
bugs: review+
mats: superreview+
mconnor: approval1.8.1.6+
mconnor: approval1.8.1.8+
dveditz: approval1.8.0.13+
Details | Diff | Review

Description Toh Yanxi 2007-07-18 09:07:33 PDT
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.5) Gecko/20070713 Firefox/2.0.0.5
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.5) Gecko/20070713 Firefox/2.0.0.5

in the place where u enter url(s), when u click for the dropdown menu, the scrollbar can move, but the url(s) don't change. It's as if they were stuck.

Reproducible: Always

Steps to Reproduce:
1.open firefox
2.click on the dropdown menu
3.scroll
Actual Results:  
nothing happens.

Expected Results:  
display all my previously browsed addresses.
Comment 1 Adam Guthrie 2007-07-18 10:00:39 PDT
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.5) Gecko/20070713 Firefox/2.0.0.5

Confirmed regression from Firefox 2.0.0.4. I'll work on finding a regression range.
Comment 3 Stefan [:stefanh] (away until May 28) 2007-07-18 11:49:32 PDT
fwiw, I don't seem to be able to reproduce this on SeaMonkey (latest branch build).
Comment 4 Stefan [:stefanh] (away until May 28) 2007-07-18 11:50:28 PDT
(Mac OS 10.3.9)
Comment 5 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-18 12:16:11 PDT
I see it on FIrefox. This is a pretty bad branch regression.
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-18 12:40:35 PDT
(should be Mac only though, at least)
Comment 7 Olli Pettay [:smaug] 2007-07-18 15:09:03 PDT
Created attachment 272867 [details] [diff] [review]
a guess fix.

I don't have a mac, so can't test whether this works. Anyone willing to try?
Does the problem occur only when dragging scrollbar?
Comment 8 Olli Pettay [:smaug] 2007-07-18 15:22:55 PDT
If the patch fixes this, the regression is from bug 382444.
Comment 9 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-18 16:05:33 PDT
Created attachment 272879 [details] [diff] [review]
fix

This is the right fix.

The problem is that nsNativeScrollbarFrame::Hookup is called from
nsNativeScrollbarFrame::GetPrefSize. The first time it runs it interposes
itself as the scrollbar mediator: it keeps a reference to the old scrollbar
mediator, makes itself the new scrollbar mediator, and forwards messages from
the scrollbar to the old mediator.

But the fix for bug 382444 is setting the scrollbar mediator after GetPrefSize has run for the first time. So nsNativeScrollbarFrame thinks there is no mediator and doesn't install itself as a mediator, so never passes events to the tree's mediator even after the tree has installed it.

The right thing to do is to just always make the scrollframe a mediator always. Every time a mediator event happens we recheck whether there's a real mediator (this already happens in nsNativeScrollbarFrame::FindParts). We also need to make nsNativeScrollbar always perform regular scrolling *and* mediator notification.

Good thing native scrollbars are gone on trunk!

I guess basically all tree scrolling is broken on Mac in 2.0.0.5 :-(
Comment 10 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-18 16:13:53 PDT
The blame for this is mainly mine. My patch changed things so that SetScrollbarMediator has to be called before GetPrefSize is called on the scrollbar (an unintentional and stupid requirement). No-one noticed because all our code was doing that. Then Olli checked in his tree patch and violated the requirement.
Comment 11 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2007-07-18 16:43:38 PDT
Thunderbird 2.0.0.5 hasn't been released yet. I guess this is something of a show-stopper for that, right?
Comment 12 Marcia Knous [:marcia - use ni] 2007-07-18 17:10:19 PDT
roc: Tomcat and I don't see scrolling broken on the Thunderbird 2.0.0.5pre build. We are able to scroll the message pane as well as the account pane (not great experience but the scroll bar chunks through). Since this is core code I guess we are puzzled why it wouldn't break in both products. Can you enlighten us?

(In reply to comment #9)
> Created an attachment (id=272879) [details]
> fix
> 
> This is the right fix.
> 
> The problem is that nsNativeScrollbarFrame::Hookup is called from
> nsNativeScrollbarFrame::GetPrefSize. The first time it runs it interposes
> itself as the scrollbar mediator: it keeps a reference to the old scrollbar
> mediator, makes itself the new scrollbar mediator, and forwards messages from
> the scrollbar to the old mediator.
> 
> But the fix for bug 382444 is setting the scrollbar mediator after GetPrefSize
> has run for the first time. So nsNativeScrollbarFrame thinks there is no
> mediator and doesn't install itself as a mediator, so never passes events to
> the tree's mediator even after the tree has installed it.
> 
> The right thing to do is to just always make the scrollframe a mediator always.
> Every time a mediator event happens we recheck whether there's a real mediator
> (this already happens in nsNativeScrollbarFrame::FindParts). We also need to
> make nsNativeScrollbar always perform regular scrolling *and* mediator
> notification.
> 
> Good thing native scrollbars are gone on trunk!
> 
> I guess basically all tree scrolling is broken on Mac in 2.0.0.5 :-(
> 

Comment 13 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-18 17:44:21 PDT
Did you test on Mac? This is Mac only. If it works on Mac then I'm confused.
Comment 14 Marcia Knous [:marcia - use ni] 2007-07-18 17:45:56 PDT
Yes, I tested on an Intel Mac running 10.4.10.

(In reply to comment #13)
> Did you test on Mac? This is Mac only. If it works on Mac then I'm confused.
> 

Comment 15 Stephen Donner [:stephend] - PTO; back on 5/28 2007-07-18 18:24:42 PDT
This also affects the "Message Filters" dialog in Thunderbird on Mac (2.0.0.5 candidates), if you overflow the message criteria, and try to scroll in a folder picket which has scrollbars, like the "Move message to" criterion.
Comment 16 Dave Miller [:justdave] (justdave@bugzilla.org) 2007-07-18 19:56:51 PDT
Preed told me to mention this here....

This affects the Password Manager also on Mac...

I renamed a username for a login account on a site I manage tonight, and then logged out and logged back in to try to save the new username in password manager.  Then I went to the preferences and into Password Manager to attempt to delete the old username since it'll never be used again, and the scrollbar seemed to start scrolling then stopped, and wouldn't start scrolling again.  So I never did get to that username to delete it.
Comment 17 Dave Miller [:justdave] (justdave@bugzilla.org) 2007-07-18 20:08:40 PDT
ah, to clarify, the scrollbar itself works, but the content stops moving.
Comment 18 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-18 20:17:13 PDT
You can probably select an entry and then use keyboard navigation to get down to the one you want. Mousewheel also works if you have one.

I'm just saying that for completeness .... this regression sucks.
Comment 19 Olli Pettay [:smaug] 2007-07-18 23:27:11 PDT
Comment on attachment 272879 [details] [diff] [review]
fix

>Index: widget/src/mac/nsNativeScrollbar.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/widget/src/mac/nsNativeScrollbar.cpp,v
>retrieving revision 1.12.4.2
>diff -u -p -1 -2 -r1.12.4.2 nsNativeScrollbar.cpp
>--- widget/src/mac/nsNativeScrollbar.cpp	8 May 2006 20:53:48 -0000	1.12.4.2
>+++ widget/src/mac/nsNativeScrollbar.cpp	18 Jul 2007 23:05:09 -0000
>@@ -229,31 +229,29 @@ nsNativeScrollbar::DoScrollAction(Contro
>       return;
>   }
> 
>   if (buttonPress) {
>     //
>     // For the up/down buttons, scroll up or down by the line height and 
>     // update the attributes on the content node (the scroll frame listens
>     // for these attributes and will scroll accordingly). However,
>     // if we have a mediator, we're in an outliner and we have to scroll by
>     // lines. Outliner ignores the indexes in ScrollbarButtonPressed() except
>     // to check if one is greater than the other to indicate direction.
>     //
>+    UpdateContentPosition(newPos);
>     if (mMediator) {
>       BoundsCheck(0, newPos, mMaxValue);
>       mMediator->ScrollbarButtonPressed(mScrollbar, oldPos, newPos);
>     }
>-    else {
>-      UpdateContentPosition(newPos);
>-    }
>   }
>   else {
>     //
>     // For page up/down and dragging the thumb, scroll by the page height
>     // (or directly report the value of the scrollbar) and update the attributes
>     // on the content node (as above). If we have a mediator, we're in an
>     // outliner so tell it directly that the position has changed. Note that
>     // outliner takes the new position as a signed reference, so we have to
>     // convert our unsigned to signed first.
>     //
>     UpdateContentPosition(newPos);

Couldn't you have just one UpdateContentPosition(newPos), right before |if (buttonPress)|?
Comment 20 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-19 01:26:15 PDT
Yes.
Comment 21 Stephen Donner [:stephend] - PTO; back on 5/28 2007-07-19 01:45:11 PDT
*** Bug 388734 has been marked as a duplicate of this bug. ***
Comment 22 Robert Kaiser (not working on stability any more) 2007-07-19 06:47:12 PDT
I added this fix to SeaMonkey 1.1.3 (I did minibranch the files for that but unfortunately hit the wrong branch when wanting to check in on that minibranch, I did back out immediately - sorry!)
Comment 23 Carsten Book [:Tomcat] 2007-07-19 08:38:09 PDT
*** Bug 388798 has been marked as a duplicate of this bug. ***
Comment 24 Phil Ringnalda (:philor) 2007-07-19 09:00:02 PDT
*** Bug 388775 has been marked as a duplicate of this bug. ***
Comment 25 Mats Palmgren (:mats) 2007-07-19 09:03:33 PDT
Comment on attachment 272879 [details] [diff] [review]
fix

sr=mats
Comment 26 Stefan [:stefanh] (away until May 28) 2007-07-19 10:04:31 PDT
*** Bug 388711 has been marked as a duplicate of this bug. ***
Comment 27 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-19 15:27:56 PDT
Comment on attachment 272879 [details] [diff] [review]
fix

Need branch approvals for this major regression.
Comment 28 Phil Ringnalda (:philor) 2007-07-20 20:25:53 PDT
*** Bug 389047 has been marked as a duplicate of this bug. ***
Comment 29 Phil Ringnalda (:philor) 2007-07-20 20:26:08 PDT
*** Bug 389044 has been marked as a duplicate of this bug. ***
Comment 30 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2007-07-21 06:53:52 PDT
*** Bug 389078 has been marked as a duplicate of this bug. ***
Comment 31 Phil Ringnalda (:philor) 2007-07-21 08:23:08 PDT
*** Bug 389079 has been marked as a duplicate of this bug. ***
Comment 32 Justin Dolske [:Dolske] 2007-07-21 19:34:03 PDT
*** Bug 389124 has been marked as a duplicate of this bug. ***
Comment 33 Steve England [:stevee] 2007-07-22 08:03:23 PDT
*** Bug 389166 has been marked as a duplicate of this bug. ***
Comment 34 Mike Connor [:mconnor] 2007-07-22 13:56:31 PDT
Comment on attachment 272879 [details] [diff] [review]
fix

a=mconnor on behalf of drivers, please land on GECKO181_20070712_RELBRANCH for
2.0.0.6 and MOZILLA_1_8_BRANCH for 2.0.0.7
Comment 35 Jon Gateley 2007-07-22 14:31:39 PDT
(In reply to comment #0)
> User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US;
> rv:1.8.1.5) Gecko/20070713 Firefox/2.0.0.5
> Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US;
> rv:1.8.1.5) Gecko/20070713 Firefox/2.0.0.5
> 
> in the place where u enter url(s), when u click for the dropdown menu, the
> scrollbar can move, but the url(s) don't change. It's as if they were stuck.
> 
> Reproducible: Always
> 
> Steps to Reproduce:
> 1.open firefox
> 2.click on the dropdown menu
> 3.scroll
> Actual Results:  
> nothing happens.
> 
> Expected Results:  
> display all my previously browsed addresses.
> 

I got mine to work using the mousewheel!  You may have to jiggle the scrollbar a bit to get mousewheel to work.
Comment 36 Ryan Flint [:rflint] (ping via IRC for reviews) 2007-07-22 19:30:03 PDT
*** Bug 389207 has been marked as a duplicate of this bug. ***
Comment 37 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-22 20:07:54 PDT
Checked into GECKO181_20070712_RELBRANCH and MOZILLA_1_8_BRANCH.
Comment 38 Carsten Book [:Tomcat] 2007-07-23 07:34:14 PDT
verified fixed 1.8.1.6 using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.6) Gecko/2007072301 Firefox/2.0.0.6 autocomplete dropdown/scrollbar works now as expected. Adding verified keyword
Comment 39 JOHN DIELE 2007-07-23 09:21:35 PDT
MIGHT BE NICE IF FIREFOX FIXED IT.  ALL THESE CC'S ARE GREEK AND SERVE NO PURPOSE.
Comment 40 Russell Gillette 2007-07-23 09:55:57 PDT
(In reply to comment #38)
> verified fixed 1.8.1.6 using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US;
> rv:1.8.1.6) Gecko/2007072301 Firefox/2.0.0.6 autocomplete dropdown/scrollbar
> works now as expected. Adding verified keyword
> 

what about PPC-based Mac?
Comment 41 Marcia Knous [:marcia - use ni] 2007-07-23 10:35:28 PDT
Verified fixed on PPC using Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.6) Gecko/2007072301 Firefox/2.0.0.6. Scrollbars work lovely in the URL bar and elsewhere.
> 
> what about PPC-based Mac?
> 

Comment 42 Stephen Donner [:stephend] - PTO; back on 5/28 2007-07-23 17:31:32 PDT
*** Bug 389335 has been marked as a duplicate of this bug. ***
Comment 43 Justin Dolske [:Dolske] 2007-07-24 01:35:32 PDT
*** Bug 389371 has been marked as a duplicate of this bug. ***
Comment 44 Ria Klaassen (not reading all bugmail) 2007-07-26 07:59:45 PDT
*** Bug 389657 has been marked as a duplicate of this bug. ***
Comment 45 Chris F. 2007-07-26 13:46:20 PDT
Alright in all honesty this is greek to most of us. How do you implement the fix, alot of us are having issues because we don't know how to, so how about a walk through on that.
Comment 46 Robert Kaiser (not working on stability any more) 2007-07-26 13:53:03 PDT
Chris:
If you don't compile builds yourself, then either go back to Firefox 2.0.0.4 (which is bad as it has security issues), wait for Firefox 2.0.0.6, which should be available soon (there might test builds somewhere already), or use SeaMonkey 1.1.3 which has the fix (not that I want to force anyone to convert from Firefox to SeaMonkey here though).
Your best option is probably to wait for Firefox 2.0.0.6 which should be available soon.
Comment 47 Chris F. 2007-07-26 13:55:09 PDT
Robert,

Thanks. I've compiled builds, but never for firefox and never for a mac. If 2.0.0.6 will be out soon I can just wait. Thanks for the help.

Chris
Comment 48 Carsten Book [:Tomcat] 2007-07-28 16:03:52 PDT
*** Bug 389980 has been marked as a duplicate of this bug. ***
Comment 49 philippe (part-time) 2007-07-28 22:16:16 PDT
*** Bug 390005 has been marked as a duplicate of this bug. ***
Comment 50 Daniel Veditz [:dveditz] 2007-08-07 11:22:41 PDT
Comment on attachment 272879 [details] [diff] [review]
fix

approved for 1.8.0.13, a=dveditz for release-drivers
Comment 51 Daniel Veditz [:dveditz] 2007-08-09 12:39:20 PDT
Fix checked into 1.8.0.13
Comment 52 juan becerra [:juanb] 2007-08-22 15:59:44 PDT
Verified on MacIntel using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.7pre) Gecko/2007082105 BonEcho/2.0.0.7pre, and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.7pre) Gecko/2007082103 Thunderbird/2.0.0.7pre

Adding verified keyword.

I was not able to observe the problem on Thunderbird 15012 or 15013. 
Comment 53 Marcia Knous [:marcia - use ni] 2008-12-31 15:18:27 PST
minusing my original nomination for in-litmus as I don't feel we need a test case for this.

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