Australis tabs OS X lightweight theme support

RESOLVED FIXED in Firefox 28

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: MattN, Assigned: MattN)

Tracking

Trunk
Firefox 28
All
macOS
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Australis:M3])

Attachments

(3 attachments, 4 obsolete attachments)

Lightweight theme styling is needed for OS X similar to bug 823180 for Windows.  The main change involves keeping borders between the navbar and tabs when a theme is applied.

Stephen, since we use a solid color for for the selected tab on OS X, I wasn't sure what color and opacity should be used on the selected tab and toolbars below when lightweight themes were applied. Can you think of other differences from the Windows LWT approach? Thanks! Otherwise I have this mostly working.
Flags: needinfo?(shorlander)
That should have been bug 813786 in comment 0 for Windows LWT support in Australis.
Posted image Persona OSX - Light
It should look pretty much the same WRT to the tab strip.
Flags: needinfo?(shorlander)
IIUC, shorlander said we can use the same color and opacity as Windows for the toolbar and selected tab.
Keywords: uiwanted
Note the copy that Splinter will not show properly:
copy from browser/themes/windows/browser-lightweightTheme.css
copy to browser/themes/osx/browser-lightweightTheme.css

I got rid of the overlapping 2px of toolbars because:
* the overlap became visible with the LWT semi-transparent overlay
* it's simpler IMO
* aligns with the Windows implementation
Attachment #745503 - Flags: review?(mconley)
Attachment #745503 - Flags: review?(dao)
Comment on attachment 745503 [details] [diff] [review]
v.1 Simplify toolbar borders and prevent overlapping

How are the animation perf numbers on OS X?
Posted image Latest patch applied to UX (obsolete) —
This patch has bitrotted. After manually de-bitrotting, this is what I'm getting with a lw-theme applied. Something is up with this patch...
Comment on attachment 745503 [details] [diff] [review]
v.1 Simplify toolbar borders and prevent overlapping

Please see the screenshot I just posted.
Attachment #745503 - Flags: review?(mconley) → review-
(In reply to Mike Conley (:mconley) from comment #7)
> This patch has bitrotted. After manually de-bitrotting, this is what I'm
> getting with a lw-theme applied. Something is up with this patch...

I'm rebuilding on top of latest UX so I haven't seen the problem from the screenshot yet but the patch applied without fuzz for me.
I am definitely getting patch fuzz using a recent pull of UX:

Mikes-MacBook-Pro:ux mikeconley$ hg qimport https://bugzilla.mozilla.org/attachment.cgi?id=745503 -n 865178
adding 865178 to series file

Mikes-MacBook-Pro:ux mikeconley$ hg qpush
applying 865178
patching file browser/themes/osx/browser.css
Hunk #1 FAILED at 0
Hunk #2 FAILED at 61
2 out of 3 hunks FAILED -- saving rejects to file browser/themes/osx/browser.css.rej
patching file browser/themes/osx/jar.mn
Hunk #1 FAILED at 14
1 out of 2 hunks FAILED -- saving rejects to file browser/themes/osx/jar.mn.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh 865178
Mikes-MacBook-Pro:ux mikeconley$
Rebased with a regression fix for bug 850918 which I believe is what was shown in the screenshot.

(In reply to Dão Gottwald [:dao] from comment #6)
> How are the animation perf numbers on OS X?

I'm doing an opt build now and will post the numbers.
Attachment #745503 - Attachment is obsolete: true
Attachment #745972 - Attachment is obsolete: true
Attachment #745503 - Flags: review?(dao)
Attachment #746013 - Flags: review?(mconley)
Attachment #746013 - Flags: review?(dao)
Perf numbers (see bottom for OS X):
https://docs.google.com/spreadsheet/pub?key=0Av64yYvszN34dDdZX0FYWEd3MVpEVzdjYXlFcGpUQmc&single=true&gid=4&output=html

Comparing m-c and UX:
Frame intervals are essentially the same across the board. It seems like bug 676241 will help a little bit too.
Paint times increased for tab open without a LWT but decreased slightly for tab open and close with a LWT. It seems like the performance is acceptable to proceed for now. Note that there seems to be more noise than with Windows.
Posted image Dragging a tab on OSX (obsolete) —
With this latest patch, dragging the tab doesn't cause anything to be drawn for the background. I'm gonna go out on a limb and assume that this isn't intentional. :)
Comment on attachment 746013 [details] [diff] [review]
v.1.1 Fix regression caused by bug 850918

See previous comment and attachment.
Attachment #746013 - Flags: review?(mconley)
This fixed the tab drag issue by making OS X work like Windows and Linux do.

I added the new define because we could assume the same gradient worked for both LWT and non-LWT on Windows/Linux but this is not the case on OS X where the gradient is opaque without a LWT.

I tested that this didn't regress Windows with or without a LWT and with scaling.
Attachment #746013 - Attachment is obsolete: true
Attachment #746415 - Attachment is obsolete: true
Attachment #746013 - Flags: review?(dao)
Attachment #747104 - Flags: review?(mconley)
Comment on attachment 747104 [details] [diff] [review]
v.2 Fix tab drag

This looks good to me.

I tested this on Ubuntu, and noticed that the selected tab background is still stretched - but then I also found out that it's that way with or without your patch. So something is still kinda screwy, but it's not this patch's fault.

I'll file a bug for the Linux theme stuff.

Great job on this.
Attachment #747104 - Flags: review?(mconley) → review+
(In reply to Matthew N. [:MattN] from comment #15)
> I added the new define because we could assume the same gradient worked for
> both LWT and non-LWT on Windows/Linux but this is not the case on OS X where
> the gradient is opaque without a LWT.

Why does the gradient need to be opaque without a LWT?
(In reply to Dão Gottwald [:dao] from comment #17)
> (In reply to Matthew N. [:MattN] from comment #15)
> > I added the new define because we could assume the same gradient worked for
> > both LWT and non-LWT on Windows/Linux but this is not the case on OS X where
> > the gradient is opaque without a LWT.
> 
> Why does the gradient need to be opaque without a LWT?

It doesn't need to be opaque but unlike Windows/Linux, the gradient doesn't need to overlay -moz-dialog. That's also how the gradient was specified in the spec and it seemed easier to maintain as one gradient instead of two layers.

It may be possible to come up with a gradient and base color such that the gradient works for the LWT and non-LWT case but the look may not be desired.
Whiteboard: [Australis:M3] → [Australis:M3][fixed-in-ux]

Comment 24

6 years ago
https://hg.mozilla.org/mozilla-central/rev/39b1b4eee94b
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M3][fixed-in-ux] → [Australis:M3]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.