Closed Bug 1452303 Opened 3 years ago Closed 3 years ago

App menu separators regressed with bug 1451711

Categories

(Firefox :: Theme, defect, P1)

defect

Tracking

()

RESOLVED FIXED
Firefox 61
Tracking Status
firefox60 --- unaffected
firefox61 --- fixed

People

(Reporter: ntim, Assigned: ntim)

References

Details

Attachments

(3 files)

Attached image Screenshot
Bug 1451711 made that the separator looks too light on mac.
Dão, how would you recommend fixing this ? Basically, since --panel-separator-color has an alpha channel, the color is now relative to the new ThreeDShadow background as opposed to the old semi-transparent background.
Flags: needinfo?(dao+bmo)
(In reply to Tim Nguyen :ntim from comment #1)
> Dão, how would you recommend fixing this ? Basically, since
> --panel-separator-color has an alpha channel, the color is now relative to
> the new ThreeDShadow background as opposed to the old semi-transparent
> background.

We should stop using background here and instead use border all the time so that --panel-separator-color completely overrides ThreeDShadow.
Flags: needinfo?(dao+bmo)
Priority: -- → P1
(In reply to Dão Gottwald [::dao] from comment #2)
> (In reply to Tim Nguyen :ntim from comment #1)
> > Dão, how would you recommend fixing this ? Basically, since
> > --panel-separator-color has an alpha channel, the color is now relative to
> > the new ThreeDShadow background as opposed to the old semi-transparent
> > background.
> 
> We should stop using background here and instead use border all the time so
> that --panel-separator-color completely overrides ThreeDShadow.

Which border(s) should be set ? There are vertical/horizontal separators depending on where toolbarseparator is used.
Flags: needinfo?(dao+bmo)
(In reply to Tim Nguyen :ntim from comment #3)
> (In reply to Dão Gottwald [::dao] from comment #2)
> > (In reply to Tim Nguyen :ntim from comment #1)
> > > Dão, how would you recommend fixing this ? Basically, since
> > > --panel-separator-color has an alpha channel, the color is now relative to
> > > the new ThreeDShadow background as opposed to the old semi-transparent
> > > background.
> > 
> > We should stop using background here and instead use border all the time so
> > that --panel-separator-color completely overrides ThreeDShadow.
> 
> Which border(s) should be set ? There are vertical/horizontal separators
> depending on where toolbarseparator is used.

Both a vertical and a horizontal border?
Flags: needinfo?(dao+bmo)
Looks like macOS also supports -moz-appearance: separator, meaning that we can simply be consistent with Windows/Linux.
Keywords: good-first-bug
Keywords: good-first-bug
Comment on attachment 8966789 [details]
Bug 1452303 - Use -moz-appearance:separator instead of background for toolbar separators.

https://reviewboard.mozilla.org/r/235466/#review241288

::: toolkit/themes/osx/global/toolbar.css:28
(Diff revision 1)
>  toolbarseparator {
> -  -moz-appearance: none;
> +  -moz-appearance: separator;
>    margin: 3px 4px;
> -  background-color: ThreeDShadow;
> -  padding: 0;
>    width: 1px !important;

Can width be dropped?
Attachment #8966789 - Flags: review?(dao+bmo) → review+
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/485b7f8c9d9a
Use -moz-appearance:separator instead of background for toolbar separators. r=dao
https://hg.mozilla.org/mozilla-central/rev/485b7f8c9d9a
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Assignee: nobody → ntim.bugs
Depends on: 1461635
See Also: → 1617920
You need to log in before you can comment on or make changes to this bug.