The page action meatball and other urlbar buttons should be hidden on about pages. It's hidden on newtab right now but not preferences, home, and probably others. Bookmarking, Pocketing, and all the other page actions don't make sense for about pages.

How's this? #identity-box gets "chromeUI" set as its class here: https://dxr.mozilla.org/mozilla-central/rev/f0abd25e1f4acced652d180c34b7c9eda638deb1/browser/base/content/browser.js#7422 The whitelist for `_isSecureInternalUI` is here: https://dxr.mozilla.org/mozilla-central/rev/f0abd25e1f4acced652d180c34b7c9eda638deb1/browser/base/content/browser.js#7056 These new selectors don't hide #userContext-icons or #urlbar-zoom-button. Both seem appropriate to keep showing IMO. I don't see this specified in the Invision spec.

Needed to update the page actions test. As a side effect, it now checks that the actions are hidden on about:home (or the main button anyway).

Comment on attachment 8900966 [details] Bug 1388835 - Hide page action urlbar buttons on about pages (about:preferences, etc.). https://reviewboard.mozilla.org/r/172416/#review177828 Very neat. Along these lines, maybe you want to fix bug 1389554 next? Note that we don't use chromeUI for all about: pages (notably not for 'blank' pages like about:newtab, where this is already hidden, but also not for various other ones, see also the comments in https://bugzilla.mozilla.org/show_bug.cgi?id=1365552 and the current whitelist here: https://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#7052 ). So I *think* reusing this list is fine here. Otherwise we'll need a separate attribute that mirrors the protocol of the current page or something. Up to you! ::: browser/base/content/test/urlbar/browser_page_action_menu.js:131 (Diff revision 2) > + Assert.equal( > + window.getComputedStyle(BrowserPageActions.mainButtonNode).visibility, > + "collapse", > + "Main button should be hidden on about:home" > + ); > + BrowserPageActions.mainButtonNode.style.visibility = "visible"; Note: for the overflow button, to avoid intermittents, we've had to wait for this to have an effect (ie for the anchor to be visible) before trying to click it, otherwise the panel refused to open. Maybe we can roll that into promisePageActionPanelOpen, which is already an async function? You can look at the head.js change in https://reviewboard.mozilla.org/r/172244/diff/1#index_header for an idea of how I did this.

(In reply to :Gijs (out until Sep 1, expect no replies to requests) from comment #6) > but also not for various other ones, see also the comments in > https://bugzilla.mozilla.org/show_bug.cgi?id=1365552 and the current > Otherwise we'll need a separate attribute that mirrors the protocol of the > current page or something. Hmmm, yeah, these buttons shouldn't appear on about:memory for example, but they do, since about:memory isn't in the whitelist. I guess that even though this patch is nice and small, it's not really the right fix. We should mirror the protocol on the urlbar or somewhere related, as you say, and then hide the buttons if it's "about". And "chrome"? And...

Here's an updated patch, but there's at least one test that failed on try because the bookmark star wasn't visible like the test expected, so I'm still tracking that down. I post this now in case you feel like it's OK to r+ the non-test parts before the end of your day today. Summary: * In URLBarSetURI, set a readablescheme attribute on the urlbar to the gURLBar.makeURIReadable().scheme of the current URI. Not sure if URLBarSetURI is the right place to do this. "scheme" alone isn't right because we want reader mode pages to be actionable presumably. * Hide the page action button for about, chrome, file, and resource URIs. * Add a new urlbar-page-action class for top-level nodes in #page-action-buttons that should be hidden when the page isn't actionable. As it is right now, there are three node types: .urlbar-icon, .urlbar-icon-wrapper, and #pageActionSeparator. Having a single class for all these makes the CSS nicer. * Modify the action-panel-opening helper function for the tests to check button visibility. Thanks for that suggestion!

This rebases on the current tree and fixes a failing bookmark test. I'll clear the review for now and push to try.

(Forgot this got r+ already, can't clear the review. Anyway, I'll ask for review again when try is OK.)

Try looks OK.

Mike, would you mind reviewing? Gijs r+'ed an earlier version of this patch, but it's changed a lot after that. Comment 9 describes what it does.

(In reply to Drew Willcoxon :adw from comment #9) > Here's an updated patch, but there's at least one test that failed on try > because the bookmark star wasn't visible like the test expected, so I'm > still tracking that down. I post this now in case you feel like it's OK to > r+ the non-test parts before the end of your day today. > > Summary: > > * In URLBarSetURI, set a readablescheme attribute on the urlbar to the > gURLBar.makeURIReadable().scheme of the current URI. Not sure if > URLBarSetURI is the right place to do this. "scheme" alone isn't right > because we want reader mode pages to be actionable presumably. > > * Hide the page action button for about, chrome, file, and resource URIs. So, fwiw, I am not reeeeaally convinced that we should never let users bookmark (or copy links) to *any* about: pages. (whereas most of the ones that have chromeUI are just easy to reach, without a bookmark anyway). If that's what UX wants, even for about: pages that aren't otherwise accessible from the UI, then OK, I guess. Users can workaround by creating a bookmark and putting the URL in manually instead, though I'm still not super sure why we should make them jump through those hoops. But I *am* convinced that we *should* let people bookmark file: pages, if that makes sense. Putting long paths into the url bar is boring, and I don't see what would be wrong with bookmarking them. Potetially relevant: bug 1393802.

(In reply to Drew Willcoxon :adw from comment #0) > The page action meatball and other urlbar buttons should be hidden on about pages. It's hidden on newtab right now but not preferences, home, and probably others. Bookmarking, Pocketing, and all the other page actions don't make sense for about pages. Only about:newtab and about:home are pages with an empty locationbar to let users directly type something in. I do not understand why I shouldn't bookmark, copy or email about:buildconfig for example. Further I would expect "Send page to device" to work on about: urls, like it is possible on moz-extension:// urls. I just noticed that I'm able to mark text on about:preferences but don't have a context menu, even I'm able to use Ctrl+C. Please just don't introduce more inconsistency.

All right, this latest patch goes back to the approach that you r+'ed, Gijs. It also incorporates a couple of improvements from the later patch: * Add a new urlbar-page-action class for top-level nodes in #page-action-buttons that should be hidden when the page isn't actionable. As it is right now, there are three node types: .urlbar-icon, .urlbar-icon-wrapper, and #pageActionSeparator. Having a single class for all these makes the CSS nicer. * Modify the action-panel-opening helper function for the tests to check button visibility. Thanks for that suggestion! I'll push this to try and then land it. If UX wants to make tweaks to which pages are actionable, we can always do that.

Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cce829b3379f Hide page action urlbar buttons on about pages (about:preferences, etc.). r=Gijs

I have reproduced this bug with Nightly 57.0a1 (2017-08-19) on Windows 8.1, 64 Bit! This bug's fix is verified on Latest Nightly 57.0a1. Build ID : 20170827100418 User Agent : Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0

I agree that this doesn't make sense for some pages, eg. about:preferences, which I had bookmarked to quickly access the proxy settings. While workarounds do exist, they don't solve the underlying issue.