The bookmark vertical separators are invisible on Mac platform

VERIFIED FIXED in Firefox 61

Status

()

defect
VERIFIED FIXED
11 months ago
11 months ago

People

(Reporter: JuliaC, Assigned: mstange)

Tracking

({regression})

61 Branch
mozilla62
All
macOS
Points:
---

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox-esr60 unaffected, firefox60 unaffected, firefox61+ verified, firefox62+ verified)

Details

(Whiteboard: [fxsearch])

Attachments

(1 attachment)

[Affected versions]:
- 62.0a1 (2018-05-14)
- 61.0b5 (20180514150347)

[Affected platforms]:
- macOS 10.13.3
- Mac OS X 10.10.5

[Steps to reproduce]:
1. Launch Firefox
2. Enable the bookmarks toolbar
3. Try to add some separators (by using the "New Separator" context menu option)

[Expected result]:
- The separators are visible and successfully added 

[Actual result]:
- The separators are invisible (blank spaces are added instead) 

[Regression range]:
- Last good revision: 964dbe5e2afe0956a3f1b08ec1c0248dfdb33820
- First bad revision: 485b7f8c9d9a6a7cc23dc70803c8cd3f169634ef
- Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=964dbe5e2afe0956a3f1b08ec1c0248dfdb33820&tochange=485b7f8c9d9a6a7cc23dc70803c8cd3f169634ef
- bug 1452303 introduced the regression
Summary: The bookmark separators are invisible on Mac platform → The bookmark vertical separators are invisible on Mac platform
Blocks: 1452303
Priority: -- → P1
Whiteboard: [fxsearch]
Component: Bookmarks & History → Theme
hey Dao, is this worth blocking on?
Flags: needinfo?(dao+bmo)
(In reply to Mike Taylor [:miketaylr] from comment #1)
> hey Dao, is this worth blocking on?

Tracking yes. Blocking probably not, but let's try hard to fix this.
Flags: needinfo?(dao+bmo)
Version: Trunk → 61 Branch
mstange, could this be a widget::cocoa bug? ntim tested -moz-appearance:separator with both orientations on Mac in attachment 8966788 [details], so I'm a bit confused. Maybe the separator is rendered but with the same color as the toolbar, making it invisible?
Flags: needinfo?(mstange)
(Assignee)

Comment 4

11 months ago
The inspector shows that the width of the separator element is zero. If I set the width to 1px, for example, then the separator shows up, and its color is sufficiently different from the toolbar color.

It's probably worth setting a non-zero size for NS_THEME_SEPARATOR in nsNativeThemeCocoa::GetMinimumWidgetSize.
Flags: needinfo?(mstange)

Updated

11 months ago
Component: Theme → Widget: Cocoa
Priority: P1 → --
Product: Firefox → Core

Updated

11 months ago
Flags: needinfo?(spohl.mozilla.bugs)

Comment 5

11 months ago
trying to determine how serious this is.
(Assignee)

Comment 6

11 months ago
It's a trivial fix, I'll do it.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Flags: needinfo?(spohl.mozilla.bugs)
Comment hidden (mozreview-request)

Comment 8

11 months ago
mozreview-review
Comment on attachment 8979991 [details]
Bug 1461635 - Enforce a minimum widget size of 1x1 for toolbar separators.

https://reviewboard.mozilla.org/r/246176/#review252310
Attachment #8979991 - Flags: review?(spohl.mozilla.bugs) → review+

Comment 9

11 months ago
Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/autoland/rev/eedb9180bc3b
Enforce a minimum widget size of 1x1 for toolbar separators. r=spohl

Comment 10

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/eedb9180bc3b
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Please request Beta approval on this when you get a chance.
Flags: qe-verify+
Flags: needinfo?(mstange)
(Assignee)

Comment 12

11 months ago
Comment on attachment 8979991 [details]
Bug 1461635 - Enforce a minimum widget size of 1x1 for toolbar separators.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1452303
[User impact if declined]: invisible separators in the bookmarks toolbar
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: it's working for me in today's nightly
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: only affects toolbar separators, which are rarely used, in an extremely simple way (enforces a minimum size of 1x1)
[String changes made/needed]: none
Flags: needinfo?(mstange)
Attachment #8979991 - Flags: approval-mozilla-beta?
Comment on attachment 8979991 [details]
Bug 1461635 - Enforce a minimum widget size of 1x1 for toolbar separators.

Simple fix for invisible bookmark separators. Approved for 61.0b9.
Attachment #8979991 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I can confirm that 61.0b9 build1 (20180528091514) and 62.0a1 (2018-05-28) are verified fixed using macOS 10.13.4 and Mac OS X 10.11.6.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.