Closed
Bug 1472751
Opened 6 years ago
Closed 6 years ago
Remove MAC_NON_BROWSER_WINDOW preprocessor directive
Categories
(Firefox :: General, enhancement)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: bgrins, Assigned: bgrins)
References
Details
Attachments
(1 file)
+++ This bug was initially created as a clone of Bug #1471734 +++ Going to spin this patch out of Bug 1471734 since the other two changesets can land independently.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•6 years ago
|
||
Was thinking to add a test to make sure the openLocation menu item is hidden in browser windows and unhidden in non-browser windows but I couldn't think of a good place to put it. This is the type of thing I would have usually dumped in general/... maybe a new folder like 'windowing' or something? Or can you think of a good existing place to put it?
Flags: needinfo?(gijskruitbosch+bugs)
Comment 3•6 years ago
|
||
mozreview-review |
Comment on attachment 8989227 [details] Bug 1472751 - Remove MAC_NON_BROWSER_WINDOW preprocessor directive; https://reviewboard.mozilla.org/r/254272/#review261124 ::: browser/base/content/browser.js:1969 (Diff revision 1) > if (element) > element.setAttribute("disabled", "true"); > } > > + // Show menus that are only visible in non-browser windows > + var shownItems = ["menu_openLocation"]; Nit: let ::: browser/base/content/browser.js:1972 (Diff revision 1) > + if (element) > + element.removeAttribute("hidden"); This is fine, though I wonder how much of this JS (and the other JS for hidden/non-hidden items) we could replace with judicious use of CSS.
Attachment #8989227 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment 4•6 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #2) > Was thinking to add a test to make sure the openLocation menu item is hidden > in browser windows and unhidden in non-browser windows but I couldn't think > of a good place to put it. This is the type of thing I would have usually > dumped in general/... maybe a new folder like 'windowing' or something? Or > can you think of a good existing place to put it? I think we just shouldn't add a test for it. People will notice it pretty quickly if it starts showing up in browser windows, and if it disappears from non-browser windows, it's basically an alias for new window or new tab, which seems like a more natural thing to do in that case anyway. I might even argue that we should get rid of the menuitem entirely... :-)
Flags: needinfo?(gijskruitbosch+bugs)
Comment hidden (mozreview-request) |
Pushed by bgrinstead@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d7750a13f9b5 Remove MAC_NON_BROWSER_WINDOW preprocessor directive;r=Gijs
Comment 7•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d7750a13f9b5
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in
before you can comment on or make changes to this bug.
Description
•