Get rid of #navigator-toolbox::before and #TabsToolbar::after on OS X

VERIFIED FIXED in Firefox 32

Status

()

Firefox
Theme
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: dao, Assigned: dao)

Tracking

unspecified
Firefox 32
All
Mac OS X
Points:
---
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: p=5 s=it-32c-31a-30b.3 [qa-])

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

4 years ago
Created attachment 8432089 [details] [diff] [review]
patch

Since #TabsToolbar doesn't have a background anymore, the #navigator-toolbox::before hack should no longer be needed. As for #TabsToolbar::after, this is basically the same as bug 1016056, except that we need to be more careful about when to draw the border.

disclaimer: This is untested as I still don't have a Mac.
Attachment #8432089 - Flags: review?(MattN+bmo)
(Assignee)

Updated

4 years ago
Flags: firefox-backlog+
Comment on attachment 8432089 [details] [diff] [review]
patch

Review of attachment 8432089 [details] [diff] [review]:
-----------------------------------------------------------------

This broke the space above the tabs in fullscreen mode. I'm still looking for other changes from a mozscreenshots comparison and I will post the photos.
Attachment #8432089 - Flags: review?(MattN+bmo) → review-
Comment on attachment 8432089 [details] [diff] [review]
patch

Review of attachment 8432089 [details] [diff] [review]:
-----------------------------------------------------------------

I didn't see any other issues but I didn't check PB yet and will do so with the next version of the patch.

Screenshots with this patch applied are at http://screenshots.mattn.ca/r/20/

::: browser/themes/osx/browser.css
@@ -2777,5 @@
> - *
> - * Because of Bug 941309, we make sure this pseudoelement always exists,
> - * but we only make it visible when we need it.
> - */
> -#navigator-toolbox::before {

Getting rid of this pseudo-element (i.e. removing content) is what caused the fullscreen regression since the size was being adjusted by spaceAboveTabbar here: https://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/browser.css?rev=2565dba570f2&mark=3949#3942
(Assignee)

Comment 3

4 years ago
Marco, please add this to the current iteration.
Flags: needinfo?(mmucci)
Added to Iteration 32.3.  Can you recommend if this bug should be [qa+] or qa-] for QA verification.
Flags: needinfo?(mmucci)
Whiteboard: p=5 → p=5 s=it-32c-31a-30b.3 [qa?]
(Assignee)

Updated

4 years ago
Whiteboard: p=5 s=it-32c-31a-30b.3 [qa?] → p=5 s=it-32c-31a-30b.3 [qa-]
(Assignee)

Comment 5

4 years ago
Created attachment 8432359 [details] [diff] [review]
patch v2
Attachment #8432359 - Flags: review?(MattN+bmo)
(Assignee)

Updated

4 years ago
Attachment #8432089 - Attachment is obsolete: true
Created attachment 8433035 [details]
v.2 before & after screenshot of the cust. mode nav-bar corner

This changes the corners at the top of the nav-bar when in cust. mode.

Both are less than ideal IMO:
Before: The corner is missing (looks rounded)
After: There is an appearance of a horizontal line where the 3px border from the right [rgba(0, 0, 0, 0.05) rgba(0, 0, 0, 0.1) rgba(0, 0, 0, 0.2)] meets the new top border of rgba(0, 0, 0, 0.2). Note how the color values differ.

I'm not sure if the after is worse than before or not. Stephen, do you have thoughts on this? Should be be at least using the same opacity for the inner part of the border? Should be using a 3px border on top too? I don't know how well that would that connect to the selected tab.
Flags: needinfo?(shorlander)
(Assignee)

Comment 7

4 years ago
(In reply to Matthew N. [:MattN] from comment #6)
> Both are less than ideal IMO:

I agree, but I think the borders not connecting at all in the before screenshot is slightly more glaring. I had to look closer at the after screenshot to make out what's wrong. Can we move this to a followup bug?
(In reply to Dão Gottwald [:dao] from comment #7)
> Can we move this to a followup bug?

I don't feel strongly about this but it seems like that means intentionally landing what some would consider to be a regression. To me, the after stands out a bit more when I'm not looking for flaws because  it appears to me like the top border is extending 2 extra pixels into the padding. The before version just seems like a poor job at border-radius.
(Assignee)

Comment 9

4 years ago
Created attachment 8433072 [details] [diff] [review]
patch v3

this should smoothen the border connection a bit
Attachment #8432359 - Attachment is obsolete: true
Attachment #8432359 - Flags: review?(MattN+bmo)
Attachment #8433072 - Flags: review?(MattN+bmo)
(Assignee)

Comment 10

4 years ago
Matt, does my latest update help with your concern? If so, can we get this landed and move any brainstorming about changing the color / thickness of these borders to a different bug?
Comment on attachment 8433072 [details] [diff] [review]
patch v3

(In reply to Dão Gottwald [:dao] from comment #10)
> Matt, does my latest update help with your concern? If so, can we get this
> landed and move any brainstorming about changing the color / thickness of
> these borders to a different bug?

Yeah, apologies for the delay, I've been distracted by other bugs.
Attachment #8433072 - Flags: review?(MattN+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/9e18c67d243e
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32

Updated

4 years ago
Status: RESOLVED → VERIFIED
(In reply to Matthew N. [:MattN] from comment #6)
> Created attachment 8433035 [details]
> v.2 before & after screenshot of the cust. mode nav-bar corner
> 
> This changes the corners at the top of the nav-bar when in cust. mode.
> 
> Both are less than ideal IMO:
> Before: The corner is missing (looks rounded)
> After: There is an appearance of a horizontal line where the 3px border from
> the right [rgba(0, 0, 0, 0.05) rgba(0, 0, 0, 0.1) rgba(0, 0, 0, 0.2)] meets
> the new top border of rgba(0, 0, 0, 0.2). Note how the color values differ.
> 
> I'm not sure if the after is worse than before or not. Stephen, do you have
> thoughts on this? Should be be at least using the same opacity for the inner
> part of the border? Should be using a 3px border on top too? I don't know
> how well that would that connect to the selected tab.

Neither state is ideal, I don't think the after looks any worse than the before though.
Flags: needinfo?(shorlander)
You need to log in before you can comment on or make changes to this bug.