Closed
Bug 342900
Opened 19 years ago
Closed 18 years ago
open tab in background gives no indication that tab opened in overflow area
Categories
(Firefox :: Tabbed Browser, defect)
Tracking
()
VERIFIED
FIXED
Firefox 2 beta2
People
(Reporter: vlad, Assigned: moco)
References
Details
(Keywords: verified1.8.1, Whiteboard: [mustfix])
Attachments
(3 files, 13 obsolete files)
(deleted),
patch
|
moco
:
review+
mtschrep
:
approval1.8.1+
|
Details | Diff | Splinter Review |
(deleted),
image/tiff
|
Details | |
(deleted),
image/png
|
Details |
If you have enough tabs to be scrolling the tab bar and you control-click a link to open it in a new tab in the background, you don't receive any indication that a new tab was actually opened -- all the current tabs stay exactly the same size, etc. We may need to flash or otherwise highlight the ">" button so that people know that something did in fact happen, and so they can find where their tab went.
Assignee | ||
Comment 1•19 years ago
|
||
interesting point. seeing what mconnor and beltzner thing about visually showing that "something" happened.
Assignee: nobody → sspitzer
Comment 2•19 years ago
|
||
*** Bug 342911 has been marked as a duplicate of this bug. ***
Comment 3•19 years ago
|
||
Rather than highlighting/flashing, inserting new tabs next to the opener sounds like a more decent solution.
Comment 4•19 years ago
|
||
(In reply to comment #3)
> Rather than highlighting/flashing, inserting new tabs next to the opener sounds
> like a more decent solution.
This goes along well with some ideas from bug 327562 which also suggest opening new tabs to the right of the current tab. Perhaps a decision should be made there first as it may fix this bug as well.
Assignee | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Flags: blocking-firefox2?
Target Milestone: --- → Firefox 2 beta1
Comment 5•19 years ago
|
||
(In reply to comment #3)
> Rather than highlighting/flashing, inserting new tabs next to the opener sounds
> like a more decent solution.
>
This won't help (by itself) if the current tab happens to be the rightmost tab currently displayed on the tab bar.
Comment 6•19 years ago
|
||
(In reply to comment #5)
> This won't help (by itself) if the current tab happens to be the rightmost tab
> currently displayed on the tab bar.
Of course auto-scrolling would have to happen in that case.
By the way, it's also annoying that one has no idea of how many tabs are off the visible area. But that's probably another bug?
Reporter | ||
Comment 7•19 years ago
|
||
(In reply to comment #3)
> Rather than highlighting/flashing, inserting new tabs next to the opener sounds
> like a more decent solution.
Don't make me come over there.
Assignee | ||
Comment 8•19 years ago
|
||
mconnor suggests:
a) do *not* change where the tab opens (continue to open it as the last tab)
b) do *not* ensure the new tab is visible in the scroll tab strip
c) do visually indicate (as vlad originally suggested) that the new tab was opened.
as an alternative to flashing, he suggested that we turn the arrow scroll button a different color, and then fade back to the normal color.
Comment 9•19 years ago
|
||
(In reply to comment #8)
Are there some reasons, too?
My thoughts are: If I open tabs (even in the background) I actually plan to switch to them very soon. Having to scroll all the time isn't fun at all. To me it's not only a question of indication, but also workflow. I think that's what bug 342911 was about, too, so it should be tackled here.
Reporter | ||
Comment 10•19 years ago
|
||
After a quick conversation with mconnor/belzner/a few others, one idea I had was to scroll the tab bar such that both the current tab and the new tab are both visible, if possible. So if your current tab is the rightmost tab and any new tabs would be overflow, it would keep scrolling to the left until it got to the first position.
However, if the current tab is already in the first position, then you still have this problem -- at which point I think visually highlighting or otherwise flashing the > would be good.
Comment 11•19 years ago
|
||
yes to blocking, but not needed for beta1
Flags: blocking-firefox2? → blocking-firefox2+
Target Milestone: Firefox 2 beta1 → Firefox 2 beta2
What vlad said. (Which is freakily like what I proposed to beltzner in an IRC channel a few minutes ago.)
I'm still not sure about the whole scrolling thing, tbh, but I'll let it grow on me.
Comment 13•19 years ago
|
||
(In reply to comment #12)
> I'm still not sure about the whole scrolling thing, tbh, but I'll let it grow
> on me.
>
Is there some public forum where a discussion is being held about "the whole scrolling thing", or are we past that point? I have some thoughts of my own on this, and I'm also interested in the discussion leading to the current solution.
Comment 14•19 years ago
|
||
(In reply to comment #8)
> mconnor suggests:
> as an alternative to flashing, he suggested that we turn the arrow scroll
> button a different color, and then fade back to the normal color.
This sounds like a good method to me. Question: if multiple pages are opened simultaneously (e.g., "Open Folder in Tabs", should the arrow scroll button pulse/flash for each tab that is opened, or only once? Similarly, if the indicator hasn't faded yet, and the user opens a second tab off-screen, how should this be handled?
Comment 15•18 years ago
|
||
Some discussion about this and other problems with the overflow solution is happening in bug 343251.
Comment 16•18 years ago
|
||
*** Bug 344385 has been marked as a duplicate of this bug. ***
Comment 17•18 years ago
|
||
*** Bug 344398 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 18•18 years ago
|
||
> as an alternative to flashing, he suggested that we turn the arrow scroll
> button a different color, and then fade back to the normal color.
the patch in bug #343251 takes care of this by changing the background of the new "all tabs" button to orange 3 times (every 150 ms).
Coming up with a better animation shouldn't be too difficult, once we decide what to do.
mconnor has suggested a "pulse then fade".
Updated•18 years ago
|
Whiteboard: [mustfix]
Assignee | ||
Comment 19•18 years ago
|
||
the fix for bug #343251 has landed (on the trunk) with my "place holder animation".
now it's time to implement mconnor's suggested animation:
mconnorfx: basically a very fast transition to full bright, then fade back to 0
mconnorfx: say 1.5 or 2 second to fade
saspitzer: so fast from 0 to bright, and then slow fade back to 0.
saspitzer: (talking about opacity)
mconnorfx: right
No longer blocks: tabbedbrowsing
Depends on: tabbedbrowsing, 343251
Assignee | ||
Updated•18 years ago
|
Whiteboard: [mustfix] → [mustfix][indication added to trunk, working on a better indication, per mconnor]
Assignee | ||
Comment 20•18 years ago
|
||
in another bug, mconnor writes:
> hardcoding colors is bad bad bad! what if the text color in the user theme was
> blue? need to find an nsITheme color that'd work in this case.
for my temporary indication, I have hard coded a color. mconnor suggested I use the native focus color.
Assignee | ||
Comment 21•18 years ago
|
||
a question for beltnzer / ben / mconnor: I'm currently showing the indication by flashing (soon to be pulse / fade) of the all tabs button. before the all tabs button, the plan to was to pulse / fade the right (for LTR) arrow scroll box button.
which do you prefer?
Assignee | ||
Comment 22•18 years ago
|
||
Assignee | ||
Comment 23•18 years ago
|
||
working on a simpler patch, as there are a couple bugs with the stack widget that prevent me from doing it this way.
additionally, mconnor suggests this exact animation:
"what I think we were envisioning was something like going from 0 to 100 in ~ .5 seconds, then staying at 100 for 2 seconds or so, then fading out over 1-1.5 seconds"
Assignee | ||
Comment 24•18 years ago
|
||
Attachment #230818 -
Attachment is obsolete: true
Assignee | ||
Comment 25•18 years ago
|
||
Attachment #230843 -
Attachment is obsolete: true
Assignee | ||
Comment 26•18 years ago
|
||
Assignee | ||
Comment 27•18 years ago
|
||
Attachment #230849 -
Attachment is obsolete: true
Attachment #230851 -
Attachment is obsolete: true
Assignee | ||
Comment 28•18 years ago
|
||
Attachment #230901 -
Attachment is obsolete: true
Assignee | ||
Comment 29•18 years ago
|
||
I need to double check on mac, but I think this is at a stage where it could be reviewed.
Attachment #230902 -
Attachment is obsolete: true
Attachment #230912 -
Flags: review?(mconnor)
Assignee | ||
Comment 30•18 years ago
|
||
question: should mouse over or click (or other events) on the button stop the animation?
Comment 31•18 years ago
|
||
(In reply to comment #30)
> question: should mouse over or click (or other events) on the button stop the
> animation?
Maybe mouseover or mousedown ...
If it doesn't feel right, speeding up the fade out animation would be an option rather than stopping it.
Assignee | ||
Comment 32•18 years ago
|
||
per mconnor:
Q: should events (like mouseover, click, etc) of the button stop any animation?
A: click yes, mouseover no, I think
to do this I've added a _stopAnimation() method that I call from when the animation end naturally and when we show the all tabs menupopup.
Attachment #230916 -
Flags: review?(mconnor)
Assignee | ||
Updated•18 years ago
|
Attachment #230912 -
Attachment is obsolete: true
Attachment #230912 -
Flags: review?(mconnor)
Assignee | ||
Comment 33•18 years ago
|
||
I haven't figure out how to reproduce this, but at least twice now I am seeing a weird issue.
the issue is that the button appears as if the opacity of the toolbarbutton is 1.0 (instead of the .999 that I've set in browser.css)
domi tells me that it's still .999, but visually, it looks like it looks when it is 1.0
Assignee | ||
Comment 34•18 years ago
|
||
Comment on attachment 230916 [details] [diff] [review]
stop animation on click
there is a problem with my patch, updated version coming....
Attachment #230916 -
Flags: review?(mconnor)
Assignee | ||
Comment 35•18 years ago
|
||
Attachment #230916 -
Attachment is obsolete: true
Attachment #231043 -
Flags: review?(mconnor)
Assignee | ||
Comment 36•18 years ago
|
||
this patch includes simon's fix for bug #345868 (which has landed on the trunk) and my fix for bug #346172 (which has also landed on the trunk.)
here's what caused the mixup over #345868. I was marking the animation as started (this.mAnimateStep = 0;) before the check to see if the tab was not visible.
simon's fix for bug #345868 exposed this issue with my patch. (thanks simon!)
mconnor, can you review?
Assignee | ||
Comment 37•18 years ago
|
||
> domi tells me that it's still .999, but visually, it looks like it looks when
it is 1.0
I still have not figured out what causes this, or how to fix it.
Assignee | ||
Comment 38•18 years ago
|
||
some feedback on the animation from asaf:
1) he feels 2.75 seconds is too long.
2 he thinks that mousdown on the scrollbutton-down button should also stop the animation.
3) he thinks there's also some sense in flashing the scrollbutton instead of the "all tabs" button
I agree with #2. The others I'm still thinking over.
beltzner / mconnor / ben?
Assignee | ||
Comment 39•18 years ago
|
||
>> domi tells me that it's still .999, but visually,
>>it looks like it looks when it is 1.0
>
>I still have not figured out what causes this, or how to fix it.
I don't have a way to reproduce it, but keep running into it.
I claim it is related to bug #346035, so I have spun out this issue into bug #346307, which I think should block ff2.
Assignee | ||
Comment 40•18 years ago
|
||
Attachment #231043 -
Attachment is obsolete: true
Attachment #231043 -
Flags: review?(mconnor)
Assignee | ||
Comment 41•18 years ago
|
||
Comment on attachment 231161 [details] [diff] [review]
uses dbaron's .99 fix for the animation (to work around bug #346307)
note, this patch includes fixes for this (and bugs #346314 #345868 #346172, which are due to land on the branch.)
Attachment #231161 -
Flags: review?(mconnor)
Assignee | ||
Comment 42•18 years ago
|
||
Attachment #231161 -
Attachment is obsolete: true
Attachment #231161 -
Flags: review?(mconnor)
Assignee | ||
Updated•18 years ago
|
Whiteboard: [mustfix][indication added to trunk, working on a better indication, per mconnor] → [mustfix][awaiting review]
Assignee | ||
Comment 43•18 years ago
|
||
Attachment #231182 -
Attachment is obsolete: true
Attachment #231321 -
Flags: review?(mconnor)
Assignee | ||
Comment 44•18 years ago
|
||
> 2 he thinks that mousedown on the scrollbutton-down button should also stop
> the animation.
spun off to bug #346581
Blocks: 346581
Assignee | ||
Comment 45•18 years ago
|
||
Attachment #231321 -
Attachment is obsolete: true
Attachment #231323 -
Flags: review?(mconnor)
Attachment #231321 -
Flags: review?(mconnor)
Comment 46•18 years ago
|
||
Comment on attachment 231323 [details] [diff] [review]
oops, forgot the flex on the hbox in the pinstripe globalBinding
>? diff.txt
>? themes/winstripe/global/jar.mn.
>Index: content/widgets/tabbrowser.xml
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/content/widgets/tabbrowser.xml,v
>retrieving revision 1.103.2.70
>diff -p -u -8 -r1.103.2.70 tabbrowser.xml
>--- content/widgets/tabbrowser.xml 30 Jul 2006 17:30:18 -0000 1.103.2.70
>+++ content/widgets/tabbrowser.xml 30 Jul 2006 19:10:39 -0000
>@@ -2481,24 +2481,36 @@
> </binding>
>
> <binding id="tabbrowser-tabs"
> extends="chrome://global/content/bindings/tabbox.xml#tabs">
> <content>
> <xul:arrowscrollbox anonid="arrowscrollbox" orient="horizontal" flex="1" style="min-width: 1px;" clicktoscroll="true">
> <children includes="tab"/>
> </xul:arrowscrollbox>
>- <xul:hbox class="tabs-alltabs-box" align="center" pack="end"
>- anonid="alltabs-box">
>- <xul:toolbarbutton class="tabs-alltabs-button" type="menu">
>- <xul:menupopup class="tabs-alltabs-popup"
>- anonid="alltabs-popup"
>- position="after_end"/>
>+ <xul:stack align="center" pack="end">
>+ /* XXXsspitzer hack
>+ * this extra hbox with position: relative
>+ * is needed to work around two bugs, see bugs #346307 and #346035
>+ */
We typically use HTML style comments, I'm surprised this works, actually.
> <xul:hbox class="tabs-closebutton-box" align="center" pack="end" anonid="tabstrip-closebutton">
> <xul:toolbarbutton class="close-button tabs-closebutton"/>
> </xul:hbox>
> </content>
> <implementation implements="nsITimerCallback">
> <constructor>
> <![CDATA[
> var pb2 =
>@@ -2535,19 +2547,19 @@
> }
> window.addEventListener("resize", onResize, false);
> ]]>
> </constructor>
>
> <destructor>
> <![CDATA[
> // Release timer to avoid reference cycles.
>- if (this.mFlashTimer) {
>- this.mFlashTimer.cancel();
>- this.mFlashTimer = null;
>+ if (this.mAnimateTimer) {
>+ this.mAnimateTimer.cancel();
>+ this.mAnimateTimer = null;
> }
> ]]>
> </destructor>
>
> <field name="mTabstripWidth">0</field>
>
> <field name="mTabstrip">
> document.getAnonymousElementByAttribute(this, "anonid", "arrowscrollbox");
>@@ -2680,67 +2692,87 @@
> "anonid", "alltabs-popup");
> </field>
>
> <field name="mAllTabsBox">
> document.getAnonymousElementByAttribute(this,
> "anonid", "alltabs-box");
> </field>
>
>- <field name="mFlashTimer">null</field>
>- <field name="mFlashStage">0</field>
>- <field name="mFlashStart">6</field>
>- <field name="mFlashDelay">150</field>
>+ <field name="mAnimateTimer">null</field>
>+ <field name="mAnimateStep">-1</field>
>+ <field name="mAnimateDelay">50</field>
>+ <field name="mAnimatePercents">
>+ [0.50, 0.60, 0.70, 0.80, 0.90,
>+ 1.00, 1.00, 1.00, 1.00, 1.00, 1.00, 1.00, 1.00, 1.00, 1.00,
>+ 1.00, 1.00, 1.00, 1.00, 1.00, 1.00, 1.00, 1.00, 1.00, 1.00,
>+ 1.00, 1.00, 1.00, 1.00, 1.00, 1.00, 1.00, 1.00, 1.00, 1.00,
>+ 0.95, 0.90, 0.85, 0.80, 0.75, 0.70, 0.65, 0.60, 0.55, 0.50,
>+ 0.45, 0.40, 0.35, 0.30, 0.25, 0.20, 0.15, 0.10, 0.05, 0.00]
>+ </field>
So, there's a cleaner way of doing this, IMO, but this will work for now.
These should be private fields (_ not m), IMO.
Looks good, let's get this on trunk and get more feedback ASAP.
Attachment #231323 -
Flags: review?(mconnor) → review+
Assignee | ||
Comment 47•18 years ago
|
||
thanks for the review.
> We typically use HTML style comments, I'm surprised this works, actually.
whoops! it works, but using DOMi, I see my comments are showing up as #text nodes.
I'll fix it.
> So, there's a cleaner way of doing this, IMO, but this will work for now.
Let me know what you have in mind, and I can clean it up. But I have found that this way makes it very easy change the animation.
> These should be private fields (_ not m), IMO.
I'll fix this, too.
i'll fix those two issues, attach a new patch, and land on the trunk.
Assignee | ||
Comment 48•18 years ago
|
||
Attachment #231323 -
Attachment is obsolete: true
Attachment #231419 -
Flags: review+
Assignee | ||
Comment 49•18 years ago
|
||
my last patch works on branch mac, trunk and branch windows, but not trunk mac!
even though the opacity of tabs-alltabs-box is 0.0 (and I've confirmed that with the DOMi) the tells, the background-color (orange, until the 2.0 theme update) always shows through.
I'll come up with a test case, and double check that it is not just a pilot error on my part.
(the patches in this bug have worked on the trunk mac before, honest!)
Comment 50•18 years ago
|
||
(In reply to comment #49)
> my last patch works on branch mac, trunk and branch windows, but not trunk mac!
bug 325296?
you could also set visibility to 'hidden'.
Assignee | ||
Comment 51•18 years ago
|
||
> bug 325296?
looks likely! I'll test out the patch in that bug, and see if that fixes my problem.
thanks!
Assignee | ||
Comment 52•18 years ago
|
||
I'm not sure the cause yet, but there is definitely a difference in how opacity is handled between the trunk and branch. See the minimal test case in bug #346035 (I've attached some screen shots.)
Assignee | ||
Comment 53•18 years ago
|
||
Assignee | ||
Comment 54•18 years ago
|
||
> (see bug #346738 a.k.a. #325286)
oops, I meant aka #325296
due to that trunk mac opacity bug, mconnor suggests that I land but comment out the background-color in pinstripe/global/browser.css which will disable the animation on mac trunk, but it will still work on windows trunk (and, it will work on mac branch, but we won't be able to test this on the mac trunk.)
I'll log a new bug about "background tab animation doesn't work on mac trunk" and refer to it in pinstripe/global/browser.css
Assignee | ||
Comment 55•18 years ago
|
||
landed on trunk, but note, the animation (and the previous "flashing" behavior) is disabled for mac due to bug #325296.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [mustfix][awaiting review] → [mustfix][landed on trunk, but disabled for mac due to bug #325296]
Comment 56•18 years ago
|
||
The hover state of All Tabs Button seems to shift on Windows 2000 or XP Classic theme.
The stack right under .tabbrowser-tabs used some famous Tabbed Extensions.
The id or class needs for stack right under .tabbrowser-tabs.
.tabbrowser-tabs > stack {margin-top: -1px; margin-bottom: 1px;}
Comment 57•18 years ago
|
||
Could the tab be scrolled into view as long as that doesn't hide the opening tab?
Like:
A < B C D E F G H > I
We have the tabs A-H, where A is not visible (left outside). We open a new tab from H or E, I. At the moment it's hidden, but will show the flashing in orange (color is not used anywhere else?).
It would make sense to scroll a little bit to the right to make the new tab visible. (according to a usability study by Google and Netscape)
!Except! if that would scroll the calling tab out of view.
How would you like that?
Assignee | ||
Comment 58•18 years ago
|
||
Comment on attachment 231419 [details] [diff] [review]
use html style comments and switch from mAnimate* to _animate, per mconnor
seeking to land on the branch to get more feedback (especially mac, as mac trunk users can't see the animation.)
(note improved animation coming soon, see bug #346901)
Attachment #231419 -
Flags: approval1.8.1?
Comment 59•18 years ago
|
||
Comment on attachment 231419 [details] [diff] [review]
use html style comments and switch from mAnimate* to _animate, per mconnor
approved by schrep for drivers
Attachment #231419 -
Flags: approval1.8.1? → approval1.8.1+
Assignee | ||
Comment 60•18 years ago
|
||
fix landed on the branch.
Blocks: 346901
Keywords: fixed1.8.1
Whiteboard: [mustfix][landed on trunk, but disabled for mac due to bug #325296] → [mustfix]
Comment 61•18 years ago
|
||
verified with Bon Echo from 20060080304 on Windows
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1 → verified1.8.1
Comment 62•18 years ago
|
||
Put Comment #57 in new bug 347315.
Comment 63•18 years ago
|
||
*** Bug 348326 has been marked as a duplicate of this bug. ***
You need to log in
before you can comment on or make changes to this bug.
Description
•