Closed Bug 1052534 Opened 7 years ago Closed 7 years ago

[10.10] Update back/forward keyhole per design for Yosemite

Categories

(Firefox :: Theme, defect)

x86
macOS
defect
Not set
normal
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 35
Iteration:
35.1
Tracking Status
firefox32 --- wontfix
firefox33 --- wontfix
firefox34 --- verified
firefox35 --- verified

People

(Reporter: Gijs, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 2 obsolete files)

The design in attachment 8440728 [details] shows an OSX-10.10-like (but with border) flat style for the back button. We should probably use a similar style for the forward button, too, and update both for 10.10.
Points: --- → 2
QA Whiteboard: [qa+]
Flags: firefox-backlog+
QA Whiteboard: [qa+]
Flags: qe-verify+
Michael, per the discussion on vidyo, can you add a mock/design that includes the forward button as well?
Flags: needinfo?(mmaslaney)
The forward button will share the same style as the Awesomebar with the addition of a right-stroke, which creates a divide between the forward and URL functionality.

Mock:
http://people.mozilla.org/~mmaslaney/firefox/Australis-OSX-Yosemite-forward
Flags: needinfo?(mmaslaney)
Probably doesn't really make sense to try to update the fwd button here without the patches from bug 1050807...
Depends on: 1050807
Marco, can you add this for this iteration? Thanks!
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Iteration: --- → 35.1
(In reply to :Gijs Kruitbosch from comment #4)
> Marco, can you add this for this iteration? Thanks!

Let's try that /with/ a needinfo...
Flags: needinfo?(mmucci)
Apologies for the delay, added to IT 35.1

(In reply to :Gijs Kruitbosch from comment #4)
> Marco, can you add this for this iteration? Thanks!
Flags: needinfo?(mmucci)
This seems good enough to me, but I'd appreciate thoughts considering there isn't a complete spec here and we can't do exactly what OS X does. Jared, you have a partition with yosemite, right? What do you think of this?
Attachment #8487456 - Flags: review?(jaws)
Comment on attachment 8487456 [details] [diff] [review]
update back/fwd keyhole design on yosemite,

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

This is mostly here, but the border looks darker on the top than on the bottom. If we can't get a uniform border around the whole back button, we should make the bottom border darker than the top.

Darker on bottom and lighter on top will match the borders of the urlbar and searchbar, as well as naturally found lighting shadows where light normally comes top-down (hence putting darker colors on the bottom and lighter on the top).

I just attached a screenshot of what it looks like on my machine.
Attachment #8487456 - Flags: review?(jaws) → review+
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #9)
> Comment on attachment 8487456 [details] [diff] [review]
> update back/fwd keyhole design on yosemite,
> 
> Review of attachment 8487456 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This is mostly here, but the border looks darker on the top than on the
> bottom. If we can't get a uniform border around the whole back button, we
> should make the bottom border darker than the top.
> 
> Darker on bottom and lighter on top will match the borders of the urlbar and
> searchbar, as well as naturally found lighting shadows where light normally
> comes top-down (hence putting darker colors on the bottom and lighter on the
> top).
> 
> I just attached a screenshot of what it looks like on my machine.

Hmm. This is the result of the urlbar clip path, I suspect - this was the case earlier, and is probably the case still. I would look at this some more to see if it can be tweaked, but I'm a little nervous about doing so considering that it seems to work fine for the other cases (ie non-Yosemite, lwthemes). I don't know why it'd behave differently here, because the CSS used is approximately the same as that for lwthemes. I guess the only difference might be the stark white backgrounds instead of the transparency that we use for lwthemes? Thoughts?
(In reply to :Gijs Kruitbosch from comment #10)
> Hmm. This is the result of the urlbar clip path, I suspect - this was the
> case earlier, and is probably the case still. I would look at this some more
> to see if it can be tweaked, but I'm a little nervous about doing so
> considering that it seems to work fine for the other cases (ie non-Yosemite,
> lwthemes). I don't know why it'd behave differently here, because the CSS
> used is approximately the same as that for lwthemes. I guess the only
> difference might be the stark white backgrounds instead of the transparency
> that we use for lwthemes? Thoughts?
Flags: needinfo?(jaws)
I fixed the issue you identified by making the styling match the lwtheme case more closely. Please check if this looks better.
Attachment #8487848 - Flags: review?(jaws)
Attachment #8487456 - Attachment is obsolete: true
Flags: needinfo?(jaws)
Actually, the forward button with lwthemes looks to be one or maybe two pixels too short (doesn't reach the bottom). I don't see this same issue without lwthemes.
Attachment #8487848 - Flags: review?(jaws) → feedback+
Again, kind of improvising a bit here.. how is this?
Attachment #8487991 - Flags: review?(jaws)
Attachment #8487848 - Attachment is obsolete: true
Comment on attachment 8487991 [details] [diff] [review]
update back/fwd keyhole design on yosemite,

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

This looks a lot better. Thanks!

There is one tiny weird thing that happens with a lightweight theme applied when I hold down the forward button to see the back/forward popup. The bottom border of the forward button disappears until the popup appears. I'm not sure if this is the result of a :hover:active style being applied and then not applied once the popup is shown.

If you can't figure that out, I'm fine with you landing this patch as-is.
Attachment #8487991 - Flags: review?(jaws) → review+
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #15)
> Comment on attachment 8487991 [details] [diff] [review]
> update back/fwd keyhole design on yosemite,
> 
> Review of attachment 8487991 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks a lot better. Thanks!
> 
> There is one tiny weird thing that happens with a lightweight theme applied
> when I hold down the forward button to see the back/forward popup. The
> bottom border of the forward button disappears until the popup appears. I'm
> not sure if this is the result of a :hover:active style being applied and
> then not applied once the popup is shown.
> 
> If you can't figure that out, I'm fine with you landing this patch as-is.

The border reappearing looks that way because of the shadow of the popup. I'll just land with keeping the border (box-shadow, but w/e) at the bottom even for the button's active state.
remote:   https://hg.mozilla.org/integration/fx-team/rev/6ff1afaaf02f
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/6ff1afaaf02f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 35
QA Contact: cornel.ionce
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:33.0) Gecko/20100101 Firefox/33.0

Verified fixed on latest Nightly, build ID: 20140914030209.
Status: RESOLVED → VERIFIED
Comment on attachment 8487991 [details] [diff] [review]
update back/fwd keyhole design on yosemite,

Approval Request Comment
[Feature/regressing bug #]: Yosemite
[User impact if declined]: we look a little out of place on 10.10, rumoured to be released in October
[Describe test coverage new/current, TBPL]: n/a, style only
[Risks and why]: low, CSS-only, has baked for a considerable amount of time on Nightly, and all in -moz-mac-yosemite-theme media queries
[String/UUID change made/needed]: none
Attachment #8487991 - Flags: approval-mozilla-aurora?
Comment on attachment 8487991 [details] [diff] [review]
update back/fwd keyhole design on yosemite,

Aurora+

Note that given the reported timing of the Yosemite release, it is likely that Firefox 33 will be the current release when Yosemite goes out. This change will ship in Firefox 34.
Attachment #8487991 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:34.0) Gecko/20100101 Firefox/34.0

Verified fixed on Mac OS X 10.9.5 using latest Aurora, build ID: 20141006004057.
My bad, Mac OS X version is 10.10
You need to log in before you can comment on or make changes to this bug.