Closed
Bug 1362281
Opened 8 years ago
Closed 8 years ago
Forward button looks different
Categories
(Firefox :: Theme, defect, P2)
Tracking
()
RESOLVED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox53 | --- | unaffected |
firefox54 | --- | unaffected |
firefox55 | + | fixed |
People
(Reporter: mstange, Assigned: shorlander)
References
Details
(Keywords: regression)
Attachments
(2 files)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
johannh
:
review+
|
Details | Diff | Splinter Review |
[Tracking Requested - why for this release]:
See the screenshot. I think this looked different before, so it's probably an unintended regression.
Comment 1•8 years ago
|
||
This was caused by consolidating the button styling to use the Windows style. The urlbar style hasn't been shared and that's conflicting a bit, I guess. We should probably find a way to make the button the same height and remove the top border again.
Comment 2•8 years ago
|
||
(In reply to Johann Hofmann [:johannh] from comment #1)
> This was caused by consolidating the button styling to use the Windows
> style. The urlbar style hasn't been shared and that's conflicting a bit, I
> guess. We should probably find a way to make the button the same height and
> remove the top border again.
It technically has the same height, the location bar just has an outer box shadow.
I think we should wontfix this. This button in its current form is going away in Firefox 57, and the different look should be acceptable for two release cycles, so it's not worth spending more time on this.
Updated•8 years ago
|
Flags: qe-verify?
Priority: P1 → P2
Reporter | ||
Comment 3•8 years ago
|
||
I think the other reason that this change is so noticeable is that the back button is less opaque when disabled than before.
Should I file a new bug for that?
(In reply to Dão Gottwald [::dao] from comment #2)
> and the different look should be acceptable for two
> release cycles
Shipping unintended visuals for two release cycles doesn't sound very desirable to me. Is it feasible to back out the changes that caused this on Beta?
Comment 4•8 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #3)
> I think the other reason that this change is so noticeable is that the back
> button is less opaque when disabled than before.
> Should I file a new bug for that?
Feel free to file a bug, but this is now consistent with what we've been doing on Windows, so this might be wontfix too.
> (In reply to Dão Gottwald [::dao] from comment #2)
> > and the different look should be acceptable for two
> > release cycles
>
> Shipping unintended visuals for two release cycles doesn't sound very
> desirable to me.
Certainly not desirable, but it's a trade-off that I think is reasonable. FWIW, Johann and I were aware of this change when Johann worked on this patch and deemed it acceptable. If course, we could be wrong. Stephen?
> Is it feasible to back out the changes that caused this on
> Beta?
No, it's too big a change and we're already making further changes on top of it.
Flags: needinfo?(shorlander)
Reporter | ||
Comment 5•8 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #4)
> (In reply to Markus Stange [:mstange] from comment #3)
> > I think the other reason that this change is so noticeable is that the back
> > button is less opaque when disabled than before.
> > Should I file a new bug for that?
>
> Feel free to file a bug, but this is now consistent with what we've been
> doing on Windows, so this might be wontfix too.
Filed bug 1362408.
Comment 6•8 years ago
|
||
Not part of photon even if we might end up doing something about this for Firefox 55.
Whiteboard: [photon-visual][p1]
Assignee | ||
Comment 7•8 years ago
|
||
It's weird enough that it's probably worth fixing. This approach seems to work.
Assignee: nobody → shorlander
Flags: needinfo?(shorlander) → needinfo?(jhofmann)
Comment 8•8 years ago
|
||
Comment on attachment 8865581 [details] [diff] [review]
fix-forward-button-macos.patch
Review of attachment 8865581 [details] [diff] [review]:
-----------------------------------------------------------------
Works for me as a temporary solution until we throw it out 57.
Thanks Stephen!
Attachment #8865581 -
Flags: review+
Comment 9•8 years ago
|
||
I'll land it for you if you like.
Status: NEW → ASSIGNED
Flags: needinfo?(jhofmann)
Comment 10•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7c6ae95fa023104532bb8fe8f0813fe8a6fa126
Bug 1362281 - Fix forward button styling for macOS. r=johannh
Comment 11•8 years ago
|
||
Comment on attachment 8865581 [details] [diff] [review]
fix-forward-button-macos.patch
Review of attachment 8865581 [details] [diff] [review]:
-----------------------------------------------------------------
::: browser/themes/osx/browser.css
@@ +587,5 @@
> + #forward-button > .toolbarbutton-icon {
> + border-top: none !important;
> + border-bottom: none !important;
> + box-shadow: none !important;
> + box-shadow: 0 .5px 0 0 rgba(0,0,0,0.2) !important;
box-shadow: none !important; can be removed.
Comment 12•8 years ago
|
||
Good point.
Comment 13•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fe0b02d865c6d02e8f5eb238c19e5ad3fb40c18
Bug 1362281 followup - Remove redundant CSS rule. r=me
Comment 14•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d7c6ae95fa02
https://hg.mozilla.org/mozilla-central/rev/7fe0b02d865c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Updated•8 years ago
|
status-firefox-esr52:
--- → unaffected
Updated•8 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•