59 bytes, text/x-review-board-request
+++ 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.
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?
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+
(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... :-)
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/d7750a13f9b5 Remove MAC_NON_BROWSER_WINDOW preprocessor directive;r=Gijs
You need to log in before you can comment on or make changes to this bug.