Closed
Bug 347783
Opened 18 years ago
Closed 18 years ago
[mac] Tab listing bar loses styling after tab overflow background animation
Categories
(Firefox :: Tabbed Browser, defect)
Tracking
()
RESOLVED
FIXED
Firefox 2 beta2
People
(Reporter: marcia, Assigned: moco)
References
Details
(Keywords: fixed1.8.1, Whiteboard: [Fx2 theme change])
Attachments
(2 files, 7 obsolete files)
17.60 KB,
patch
|
asaf
:
review+
|
Details | Diff | Splinter Review |
780 bytes,
patch
|
moco
:
review+
|
Details | Diff | Splinter Review |
Seen using Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1b1) Gecko/20060807 BonEcho/2.0b1 on my machine, Asa has seen on his Intel mac as well.
STR:
1. Make sure to have pref checked for "Always show the tab bar."
2. Open the Latest Headlines RSS feed in tabs.
3. After the feeds load, click on the first tab and then select close other tabs.
4. Observe that the styling for the tab listing bar has now disappeared.
Assignee | ||
Comment 1•18 years ago
|
||
I've seen this as well.
from the looks of it, it appears like the collaped attribute (on the tabbar) is not changing?
I'll use DOMi to confirm that theory (and get a screen shot)
Flags: blocking-firefox2?
Target Milestone: --- → Firefox 2 beta2
Assignee | ||
Comment 2•18 years ago
|
||
> from the looks of it, it appears like the collaped attribute (on the tabbar) is
> not changing?
sorry, a brain hiccup. the overflow attribute on the tab bar is what I was thinking of, and it appears to be correct.
using DOMi now to figure this one out...
Assignee | ||
Comment 3•18 years ago
|
||
the problem is this:
when you overflow, the animation will finish by setting the opacity of the alltabs-box to 0.0;
when the opacity is 0.0, you hit this bug.
Assignee | ||
Comment 4•18 years ago
|
||
from pinstripe's browser.css (see http://lxr.mozilla.org/mozilla1.8/source/toolkit/themes/pinstripe/global/browser.css#302)
Were using a background image now and the opacity varies between .65 and 1.0, depending on the state.
298 .tabs-alltabs-box{
299 width: 16px;
300 height: 25px;
301 background-image: url("chrome://global/skin/alltabs-box-bkgnd.png");
302 opacity: 0.65;
303 }
304
305 .tabs-alltabs-box:hover{
306 opacity: 1.0;
307 }
This shouldn't be here at all, it must have been a merge conflict.
309 .tabs-alltabs-box[flash="true"] {
310 background-color: orange;
311 opacity: 0.0;
312 }
on a related note, because of these .css changes, the overflow animation is broken on the mac.
I'll check in with mike to see what he wants done, and then I can fix this bug.
Assignee: nobody → sspitzer
Assignee | ||
Updated•18 years ago
|
Whiteboard: [Fx2 theme change]
Assignee | ||
Updated•18 years ago
|
Status: NEW → ASSIGNED
Summary: [mac] Tab listing bar loses styling after tab overflow → [mac] Tab listing bar loses styling after tab overflow background animation
Updated•18 years ago
|
Flags: blocking-firefox2? → blocking-firefox2+
Assignee | ||
Comment 5•18 years ago
|
||
> I'll check in with mike to see what he wants done, and then I can fix this bug.
mike wants:
1) orange on Windows, blue on OS X (use the blue we're using elsewhere in the them, like the home button's door.)
2) New effect should flash the arrow and menu.
3) It shouldn't break the style of the button"
I'm not sure if #2 applies to both mac and windows.
Comment 6•18 years ago
|
||
Two additional thoughts:
1) We might be changing the Home button's icon (see https://bugzilla.mozilla.org/show_bug.cgi?id=347602), so maybe grab a blue from the Reload button instead.
2) There's a hover state defined on the alltabs-box but it no longer works because of the reorganization of the box and button into a stack. I'm guessing that the button is now absorbing the mouseover event and so the box never gets it. Not sure if there's a way to resolve that?
Assignee | ||
Comment 7•18 years ago
|
||
> 2) There's a hover state defined on the alltabs-box but it no longer works
> because of the reorganization of the box and button into a stack. I'm guessing
> that the button is now absorbing the mouseover event and so the box never gets
> it. Not sure if there's a way to resolve that?
I see this, and I'll spin off a bug and work on it.
Assignee | ||
Comment 8•18 years ago
|
||
I chatted with dbaron and mconnor about how to do this, and here's the consensus:
I need additional "blue" (and "orange", for windows xp) versions of the following images:
http://lxr.mozilla.org/mozilla1.8/source/toolkit/themes/pinstripe/global/alltabs-box-overflow-bkgnd.png
http://lxr.mozilla.org/mozilla1.8/source/toolkit/themes/pinstripe/global/tab-arrow-end-bkgnd.png
http://lxr.mozilla.org/mozilla1.8/source/toolkit/themes/pinstripe/global/tab-arrow-start-bkgnd.png
right now, we're using the image (and the .65 opacity) on the background-image of the scrollbutton and the all-tabs box.
the plan is to have stack with the button, a box with the blue version as the background image, a box with the normal version as the background image.
note, we currently have the opacity as .65 unless we are hovering, when it is 1.0.
when animating, I would set the opacity of layer in the stack with the blue version between 0.0 and 1.0
thanks to dbaron for suggesting this approach.
I'll email jay and mconnor directly for the image request.
asaf, to accomplish this, I'll need to add a stack to the arrow scrollbox binding in scrollbox.xml. (thoughts?)
Assignee | ||
Comment 9•18 years ago
|
||
Assignee | ||
Comment 10•18 years ago
|
||
I've checked in the new images (for pinstripe only) and the only thing I need to do to keep winstripe from breaking is this change to winstripe/global/browser.css
< .tabs-alltabs-box {
---
> .tabs-alltabs-box-animate {
soon, winstripe will have a new theme refresh, and it will have this same bug.
when it does land, I can port the rest of the pinstripe/global/browser.css changes for this bug (and land the new images for winstripe that I received from jay) on the branch.
working on the hover issue now...
Assignee | ||
Comment 11•18 years ago
|
||
> 2) There's a hover state defined on the alltabs-box but it no longer works
> because of the reorganization of the box and button into a stack. I'm guessing
> that the button is now absorbing the mouseover event and so the box never gets
> it. Not sure if there's a way to resolve that?
I'm going to spin this off to another bug, so I can work on it separately.
I think to fix this I am going to need create a new binding for the alltabs-button stack (of box,box,toolbarbutton), where the toolbarbutton has handlers for mouseover and mouseout. then, on mouseover and mouseout of the toolbarbutton, I'd set fakehover to "true" on the first box (which has opacity of .65, except when in the "fakehover state", when it would be 1.0) and remove the fakehover attribute on mouseout.
the reason this has to be a binding is so I can use it from pinstripe/global/globalBindings.xml. themes are scriptable, so to get mouseover / mouseout to work, it needs to be a new binding in tabbrowser.xml
I'll discuss with mano, to see if this is a good idea or not.
Assignee | ||
Comment 12•18 years ago
|
||
> I'm going to spin this off to another bug, so I can work on it separately.
see bug #348496
Assignee | ||
Comment 13•18 years ago
|
||
Attachment #233433 -
Attachment is obsolete: true
Assignee | ||
Comment 14•18 years ago
|
||
this patch fixes this bug, animates both the scrolldown button and the alltabs button, and adds back hover to the alltabs button.
to animate the scrolldown button, I needed to extend the existing scrollbox "click to scroll" binding so that I could use a stack (box1,box2,toolbarbutton) instead of a simple toolbarbutton for the scrolldown button.
this patch includes the css changes in order to keep the hover effect on the scrolldown button (but that might be going away, see bug #347399)
I need to clean up, simplify the css in pinstripe/global/browser.css, as well as fix a couple nits that my patch introduces:
1) hover images for the scrolldown button and the alltabs button slightly off
2) dropdown for alltabs is no longer centered
Assignee | ||
Comment 15•18 years ago
|
||
Attachment #233502 -
Attachment is obsolete: true
Assignee | ||
Comment 16•18 years ago
|
||
Attachment #233584 -
Attachment is obsolete: true
Assignee | ||
Updated•18 years ago
|
Attachment #233586 -
Flags: review?(mconnor)
Assignee | ||
Comment 17•18 years ago
|
||
to elaborate on the patch:
1)
there is a new binding, arrowscrollbox-clicktoscroll-animate, which extends arrowscrollbox-clicktoscroll binding.
this new binding is in tabbrowser.xml, and not scrollbox.xml, as it is special and only really applies to tab browsing.
the new binding replaces the scrollbutton-down with a stack that contains {hbox, hbox, scrollbutton-down}. The desired
background image for the scrollbutton-down is on the first hbox, and the second hbox contains the background to show
when animating.
as with the arrowscrollbox-clicktoscroll binding, I collapse (and uncollapse) the boxes on underflow / overflow.
2)
I'm doing a similar "use stack for animation" trick {hbox, hbox, scrollbutton-down} for the alltabs button.
3) when animating, I animate both the scrollbutton-down and the alltabs button, per mconnor.
4) to fix bug #348293, have hover rules which set the background image on the toolbarbuttons (both scrollbutton-down and the alltabs)
5) I also removed the bogus ".tabs-alltabs-box[flash="true"]" rule, which I think came back with the new theme, as a result of a merge.
note, these changes don't break winstripe, even though winstripe is not refreshed yet.
I'll make a trunk version of the patch that should keep the animation working on trunk (for winstripe and pinstripe) until we land the refresh there.
Assignee | ||
Comment 18•18 years ago
|
||
Attachment #233586 -
Attachment is obsolete: true
Attachment #233695 -
Flags: review?(mconnor)
Attachment #233586 -
Flags: review?(mconnor)
Assignee | ||
Comment 19•18 years ago
|
||
notice, the trunk does not include all the pinstripe/global/browser.css changes, as those will need to be back ported to the trunk, along with the rest of the new theme.
this works on win32 trunk. things work on the mac trunk, as much as can be expected. for example, the animation color is disabled on the mac, due to bug #346738 (aka #325296)
Attachment #233711 -
Flags: review?(mconnor)
Assignee | ||
Comment 20•18 years ago
|
||
Comment on attachment 233695 [details] [diff] [review]
patch, includes fix for bug #348293 (over on all tabs) [merged with recent changes to branch for all tabs tooltip fix]
mconnor is so doomed it is not funny. switching to asaf for review.
Attachment #233695 -
Flags: review?(mconnor) → review?(bugs.mano)
Assignee | ||
Comment 21•18 years ago
|
||
Comment on attachment 233711 [details] [diff] [review]
here's a trunk version of the patch
mconnor is so doomed it is not funny. switching to asaf for review.
Attachment #233711 -
Flags: review?(mconnor) → review?(bugs.mano)
Assignee | ||
Comment 22•18 years ago
|
||
asaf says over irc:
<Mano> please use a class instead of another attribute and make the binding tabbrowser specific, something like
<Mano> .tabbrowser-tabs > .tabbrowser-arrowscrollbox
<Mano> other than that, i thought you don't need the extra hboes on trunk
<sspitzer> I thought so too, since that fix landed, but I haven't verified and tested yet.
<Mano> also, mDownBoxAnimate can be simplified
<Mano> this.mTabstrip._scrollButtonDownBoxAnimate
Assignee | ||
Comment 23•18 years ago
|
||
Attachment #233695 -
Attachment is obsolete: true
Attachment #233711 -
Attachment is obsolete: true
Attachment #233695 -
Flags: review?(bugs.mano)
Attachment #233711 -
Flags: review?(bugs.mano)
Assignee | ||
Updated•18 years ago
|
Attachment #233880 -
Flags: review?(bugs.mano)
Comment 24•18 years ago
|
||
Comment on attachment 233880 [details] [diff] [review]
updated patch, per mano's comments
failed hunks in tabbrowser.xml
Attachment #233880 -
Flags: review?(bugs.mano)
Assignee | ||
Comment 25•18 years ago
|
||
Attachment #233880 -
Attachment is obsolete: true
Assignee | ||
Comment 26•18 years ago
|
||
Comment 27•18 years ago
|
||
Comment on attachment 233895 [details] [diff] [review]
patch again
This is still almost invisible IMO, but we have a bug filed on this, I think.
Also file a follow-up on stopping the animation when the arrow button is clicked.
r=mano (branch only) with the winstripe change mentioned on IRC.
Attachment #233895 -
Flags: review+
Assignee | ||
Comment 28•18 years ago
|
||
> This is still almost invisible IMO, but we have a bug filed on this, I think.
yes, see bug #348674
> Also file a follow-up on stopping the animation when the arrow button is
> clicked.
filed it a while ago, but it is a bigger issue now that we animate that button. see bug #346581
thanks for the review, asaf.
Assignee | ||
Comment 29•18 years ago
|
||
Comment on attachment 233899 [details] [diff] [review]
a supplimental patch for winstripe
this supplimental patch has an implied r= from asaf
Attachment #233899 -
Flags: review+
Updated•18 years ago
|
Keywords: fixed1.8.1
Comment 30•18 years ago
|
||
Can this be RESOLVED FIXED, or are there still open trunk bugs here?
Assignee | ||
Comment 31•18 years ago
|
||
> Can this be RESOLVED FIXED, or are there still open trunk bugs here?
marking as fixed, as this was a theme refresh issue.
once the new theme is back ported to the trunk, if there are issues, we'll deal with them.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•