Closed
Bug 357081
Opened 19 years ago
Closed 18 years ago
tab bar doesn't resume scrolling when moving off button and back on while holding mouse button
Categories
(Firefox :: Tabbed Browser, defect)
Tracking
()
VERIFIED
FIXED
Firefox 3 alpha8
People
(Reporter: bugzilla, Assigned: dao)
References
Details
Attachments
(1 file, 3 obsolete files)
|
6.60 KB,
patch
|
Details | Diff | Splinter Review |
this occurs on:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1) Gecko/20061010 Firefox/2.0 <--- this is win XP home
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1) Gecko/20061013 Firefox/2.0 Flock/0.7.99 <--- this is danphe (trunk) this is knoppix 4.0.2
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1) Gecko/2006101304 BonEcho/2.0 <--- this is knoppix 4.0.2
STR:
Create enough new tabs that you have room to scroll around.
Click and hold arrow at either end of tab bar. Continue holding, move off arrow and back on.
Expected:
Scrolling should resume if mouse button held continuously and pointer returns to the arrow.
Actual:
Tab bar doesn't resume scrolling when moving off button and back on while holding mouse button.
|
Reporter | |
Updated•19 years ago
|
Flags: blocking-firefox2?
|
|
Reporter | |
Updated•19 years ago
|
Flags: blocking-firefox2? → blocking1.8.1.1?
|
||
Comment 1•19 years ago
|
||
Why are you nominating a minor bug to block 1.8.1.1?
|
||
Comment 2•19 years ago
|
||
Need an assignee and a patch before we could consider blocking for this.
Flags: blocking1.8.1.1? → blocking1.8.1.1-
|
Assignee | |
Comment 3•18 years ago
|
||
A fix for this should be trivial. I can do it as a follow-up to bug 347363.
Assignee: nobody → dao
Depends on: 347363
|
|
Assignee | |
Updated•18 years ago
|
Status: NEW → ASSIGNED
|
|
Assignee | |
Comment 4•18 years ago
|
||
Attachment #276941 -
Flags: review?(mano)
|
|
Assignee | |
Comment 5•18 years ago
|
||
no need to add listeners if the mouse button has already been released
Attachment #276941 -
Attachment is obsolete: true
Attachment #276942 -
Flags: review?(mano)
Attachment #276941 -
Flags: review?(mano)
|
|
Assignee | |
Updated•18 years ago
|
Attachment #276942 -
Flags: review?(mano) → review?(enndeakin)
|
||
Comment 6•18 years ago
|
||
Comment on attachment 276942 [details] [diff] [review]
patch v2
>
>+ <!-- initially empty fields:
> <field name="_scrollTimer">null</field>
>+ <field name="_mousedown">false</field>
>+ -->
Why are these commented out?
>+ <method name="handleEvent">
>+ <parameter name="aEvent"/>
>+ <body><![CDATA[
>+ if (aEvent.type == "mouseup" ||
>+ aEvent.type == "blur" && aEvent.target == document) {
&& has higher precedence, so you want parantheses here
Otherwise, looks good.
|
|
Assignee | |
Comment 7•18 years ago
|
||
(In reply to comment #6)
> (From update of attachment 276942 [details] [diff] [review])
> >
> >+ <!-- initially empty fields:
> > <field name="_scrollTimer">null</field>
> >+ <field name="_mousedown">false</field>
> >+ -->
>
> Why are these commented out?
These values are needed for |if (!this._foo)| checks only, which works with undefined properties as well. The construction of the fields can therefore be avoided, which possibly helps performance (as in attachment 270748 [details] [diff] [review], for instance).
> >+ <method name="handleEvent">
> >+ <parameter name="aEvent"/>
> >+ <body><![CDATA[
> >+ if (aEvent.type == "mouseup" ||
> >+ aEvent.type == "blur" && aEvent.target == document) {
>
> && has higher precedence
Yes, that was desired. The target check isn't needed for the mouseup event, is it?
|
|
Assignee | |
Comment 8•18 years ago
|
||
(In reply to comment #6)
> (From update of attachment 276942 [details] [diff] [review])
> >
> >+ <!-- initially empty fields:
> > <field name="_scrollTimer">null</field>
> >+ <field name="_mousedown">false</field>
> >+ -->
>
> Why are these commented out?
See also http://groups.google.com/group/mozilla.dev.performance/browse_thread/thread/6db1639b028850f5/c269064401098281
|
|
||
Comment 9•18 years ago
|
||
I meant, why are they commented out instead of just removing them?
|
|
Assignee | |
Comment 10•18 years ago
|
||
I found that useful as you can easily spot the properties that will be used, as well as their types.
|
|
Assignee | |
Comment 11•18 years ago
|
||
... then again, it's not that important to me. If that eases the review, let's ditch the comment.
Attachment #276942 -
Attachment is obsolete: true
Attachment #277515 -
Flags: review?(enndeakin)
Attachment #276942 -
Flags: review?(enndeakin)
|
|
||
Updated•18 years ago
|
Attachment #277515 -
Flags: review?(enndeakin) → review+
|
|
Assignee | |
Updated•18 years ago
|
Keywords: checkin-needed
|
|
Assignee | |
Comment 12•18 years ago
|
||
somebody check this in, please.
Attachment #277515 -
Attachment is obsolete: true
|
||
Comment 13•18 years ago
|
||
Checking in browser/base/content/tabbrowser.xml;
/cvsroot/mozilla/browser/base/content/tabbrowser.xml,v <-- tabbrowser.xml
new revision: 1.238; previous revision: 1.237
done
Checking in toolkit/content/widgets/scrollbox.xml;
/cvsroot/mozilla/toolkit/content/widgets/scrollbox.xml,v <-- scrollbox.xml
new revision: 1.25; previous revision: 1.24
done
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M8
|
|
Assignee | |
Updated•18 years ago
|
Flags: in-litmus?
|
||
Comment 14•18 years ago
|
||
Litmus Triage: marcia will handle this test case.
|
|
||
Comment 15•18 years ago
|
||
https://litmus.mozilla.org/show_test.cgi?id=5084 was added in Litmus. Verified fixed using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3pre) Gecko/2008010705 Minefield/3.0b3pre.
Status: RESOLVED → VERIFIED
Flags: in-litmus? → in-litmus+
You need to log in
before you can comment on or make changes to this bug.
Description
•