Closed
Bug 1015163
Opened 10 years ago
Closed 7 years ago
Always show menu button in popup windows
Categories
(Firefox :: Menus, defect)
Firefox
Menus
Tracking
()
VERIFIED
FIXED
Firefox 57
Tracking | Status | |
---|---|---|
firefox57 | --- | verified |
People
(Reporter: bugs, Assigned: dao)
References
(Depends on 2 open bugs, Blocks 1 open bug)
Details
Attachments
(2 files, 1 obsolete file)
46.85 KB,
image/png
|
Details | |
877 bytes,
patch
|
Details | Diff | Splinter Review |
At present, unlike IE, there is no way to show a menu to offer print/save for popups without enabling toolbar=yes which is often unwanted. Prior to Australis, under Windows, the firefox button was drawn in the window chrome, so was always available. Prior to the firefox button, enabling menubar did the job. Since the firefox button and ☰ button both offer menubar features, it seems it'd be nice if they respected that window.open hint to display themselves. Maybe even hide if menubar=no? Dunno... Also a problem in Chrome, FWIW, although Firefox and IE are the browsers over here. https://code.google.com/p/chromium/issues/detail?id=82522
Comment 1•10 years ago
|
||
So I think this (ie showing the menu button if menubar=yes or toolbar=yes) makes sense to do because we used to show the fx button pre-Australis. Dão and Madhava, do you agree? The only reason I can think of not to do this is the fact that it's kind of strange if you customize the menu to remove things to the main toolbar, they will still be invisible, but they'll be visible in the menu panel, which is a little counterintuitive. I think that might be worth it considering the fact that right now, mouse users without the menubar can't easily print from these popups anymore, and we'd be fixing that in the default case...
Flags: needinfo?(madhava)
Flags: needinfo?(dao)
Oh, this is interesting. IE9 (standard browser here) works fine, but IE11 has the exact same issue (no reasonable way to display print/save because menubar doesn't work) So, same issue as the chrome bug and this bug. Maybe a side effect of going w/ a chrome/ff layout. So even once ff and chrome fix it, still going to have to do something about IE. print buttons are easy enough, save button doable, but kind of a PITA. Here's hoping I can find a solution for IE first.
Comment 3•10 years ago
|
||
Redirecting Madhava's needinfo to Philipp, also because of the recently filed bug 1099590... Chrome has a context menu option for printing, which we've wontfixed in the past ( bug 204519), and it seems a number of users have issues with printing from popups. Yes, ideally websites shouldn't remove the toolbar/menubar in such cases... but ideally, websites would do a lot of things that they currently don't. :-( Philipp, don't know how familiar you are with window.open - for documentation on the features string, see e.g. https://developer.mozilla.org/en-US/docs/Web/API/window.open#Toolbar_and_chrome_features
Flags: needinfo?(madhava) → needinfo?(philipp)
Comment 4•10 years ago
|
||
Since it seems that we default to always show the location bar on popups, we might as well show the menu button next to it. Here's a mockup of what that could look like (showing the hover state of the menu button).
Flags: needinfo?(philipp)
Updated•10 years ago
|
Summary: consider showing the menu button for window.open'd windows without toolbar=yes but with menubar=yes → Always show menu button in popup windows
Assignee | ||
Comment 5•10 years ago
|
||
This should do it... The chained descendent selector in xul.css is pretty sad, but my only other idea would be to have browser.css explicitly set display:-moz-box for #PanelUI-popup .chromeclass-toolbar-additional, which doesn't seem great either.
Assignee: nobody → dao
Status: NEW → ASSIGNED
Flags: needinfo?(dao)
Attachment #8534307 -
Flags: review?(gijskruitbosch+bugs)
Attachment #8534307 -
Flags: review?(enndeakin)
Assignee | ||
Updated•10 years ago
|
Iteration: --- → 37.2
Points: --- → 2
Flags: qe-verify+
Flags: firefox-backlog+
Updated•10 years ago
|
Attachment #8534307 -
Flags: review?(gijskruitbosch+bugs) → review+
Updated•10 years ago
|
Attachment #8534307 -
Flags: review?(enndeakin) → review+
Assignee | ||
Comment 6•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/bc169301ff47
Comment 7•10 years ago
|
||
sorry had to back this out in https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=2a11e49c5a83 since one of this 2 changes caused test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=1424338&repo=fx-team
Flags: needinfo?(dao)
Updated•9 years ago
|
Iteration: 37.2 → 37.3
Updated•9 years ago
|
Iteration: 37.3 - 12 Jan → 38.1 - 26 Jan
Comment 8•9 years ago
|
||
Seems to me this was effectively dropped, let's only track it when you're actually spending time on it.
Iteration: 38.1 - 26 Jan → ---
Assignee | ||
Comment 9•9 years ago
|
||
Yes, I'm not only effectively but actually dropping this. I hadn't had time to figure out what's needed to make /tests/dom/html/test/test_bug369370.html happy on Windows. Whoever feels like it should feel free to pick this up. For the record, the failures were: TEST-UNEXPECTED-FAIL | /tests/dom/html/test/test_bug369370.html | Checking doc height - got 302, expected 300 TEST-UNEXPECTED-FAIL | /tests/dom/html/test/test_bug369370.html | Checking scrollTop - got 298, expected 300
Status: ASSIGNED → NEW
Flags: needinfo?(dao)
Updated•9 years ago
|
Assignee: dao → nobody
Comment 10•9 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #9) > Yes, I'm not only effectively but actually dropping this. I hadn't had time > to figure out what's needed to make /tests/dom/html/test/test_bug369370.html > happy on Windows. Whoever feels like it should feel free to pick this up. > For the record, the failures were: > > TEST-UNEXPECTED-FAIL | /tests/dom/html/test/test_bug369370.html | Checking > doc height - got 302, expected 300 > TEST-UNEXPECTED-FAIL | /tests/dom/html/test/test_bug369370.html | Checking > scrollTop - got 298, expected 300 We've seen something similar in bug 872575, unfortunately it's not really clear what fixed it there. :-(
Reporter | ||
Comment 11•9 years ago
|
||
Hrm. So this is dead due to some test fail, and a fail that somehow mysteriously fixed itself in another bug? Could it be this one is ok now?
Assignee | ||
Comment 12•7 years ago
|
||
Picking this up again. The xul.css part isn't needed anymore given recent changes to the menu button.
Comment 13•7 years ago
|
||
Pushed by dgottwald@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6c606f27cd0a Always show the menu button in popup windows. r=gijs
Assignee | ||
Comment 14•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c4a7f73f76c6a3ebd06d8eedb3795eee53d49944
Comment 15•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6c606f27cd0a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Updated•7 years ago
|
Comment 16•7 years ago
|
||
Was this decision reviewed by UX? Now with the addition of the page actions on the popup window, we have even more actions that simply don't make sense in a popup window (and open new tabs which shoudn't be happening in a popup window).
Comment 17•7 years ago
|
||
(In reply to Mike Kaply [:mkaply] from comment #16) > Was this decision reviewed by UX? Yes, see comment #4. > Now with the addition of the page actions on the popup window, we have even > more actions that simply don't make sense in a popup window (and open new > tabs which shoudn't be happening in a popup window). Mike, you've been here a long time, you know this isn't a good way to report issues. Please file new bugs for page action items that cause issues. Note also that the page action menu appearing is separate from what we do for the hamburger button. Prior to bug 748894, the bookmarks star button also showed up in the location bar in popups (just like the popup reporter button and reader mode button are also supposed to "just work" in popup windows), so we're simply being consistent with how things used to work. In principle, I also see no reason why people shouldn't be able to bookmark, pocket, copy, email or share links/pages from popup windows, so showing the page action menu makes sense to me.
Flags: needinfo?(mozilla)
Comment 18•7 years ago
|
||
I'll open new bugs for the issues I found. I still fundamentally believe we've violated the toolbar=no/menubar=no contract for popup windows.
Flags: needinfo?(mozilla)
Reporter | ||
Comment 19•7 years ago
|
||
A lot of those flags are kinda "legacy" names - but personally I'm just grateful you guys are finally offering something that honours the "yes" variant - up until now IE really was the only browser where a user could discover print who did not know the shortcut. (nevermind print preview which often has no shortcut). Also agree with him on most window options being totally legit in poups. And, frankly, if you wanna be able to disable the master button, IMO it should be a brand new popup flag.
Comment 20•7 years ago
|
||
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:57.0) Gecko/20100101 Firefox/57.0 Build ID: 20170918100059 This issue has been verified on Mac OS 10.12, Windows 8.1 x64 and Ubuntu 14.04 with latest Nightly 57.0a1 and I didn't observed any issues. I have opened multiple popup windows from which I can print, copy-paste the content and also access the hamburger menu and the page action menu from URL bar without issues.
Comment 21•7 years ago
|
||
Based on the previous comment, mark this bug Verified.
Status: RESOLVED → VERIFIED
Updated•1 year ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•