Cut and Copy menuitem are enabled when Nightly starts up
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
People
(Reporter: u520171, Unassigned)
References
Details
(6 keywords)
Attachments
(1 file, 1 obsolete file)
|
55.89 KB,
image/png
|
Details |
menuitem-enabled-disabled.png
User Agent: Mozilla/5.0 (Windows NT 6.3; rv:41.0) Gecko/20100101 Firefox/41.0 Build ID: 20150627030211 Steps to reproduce: 1) Confirm Cut and Copy menuitem are disabled or enabled after start up Firefox, Firefox Beta and Firefox Developer Edition. (Normally disabled) 2) Start up Nightly, then confirm Cut and Copy menuitem are disabled or enabled. Environments: Windows 8.1 (32 bit) / OS X 10.10 Yosemite - Firefox Nightly 41.0a1 (2015-06-26) The following build works well - Firefox Developer Edition 40.0a2 (2015-06-26) - Firefox 39.0 (2015-06-24) - Firefox 38.0.5 (2015-05-25) Actual results: Cut and Copy menuitem are enabled on Nightly. Expected results: Cut and Copy menuitem are disabled.
This problem also has occurred on the following builds. - Nightly 42.0a1 (20150629134017) - Firefox Developer Edition 41.0b2 (20150629134017)
Please refer to the attachment image (No clipboard)
|
||
Comment 4•9 years ago
|
||
Would be helpful to know exactly which Nightly build broke this. One can do this by manually downloading and testing builds from https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/, or using mozregression -- a tool to help automate this: http://mozilla.github.io/mozregression/
(In reply to Justin Dolske [:Dolske] from comment #4) > Would be helpful to know exactly which Nightly build broke this. > > One can do this by manually downloading and testing builds from > https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/, or using > mozregression -- a tool to help automate this: > http://mozilla.github.io/mozregression/ I confirmed that this problem occur on Nightly build 2015-05-27 and later. app_name: firefox build_date: 2015-05-27 build_path: k:\tmpc_9x4u\2015-05-27--mozilla-central--firefox-41.0a1.en-US.win32.zip build_txt_url: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015/05/2015-05-27-13-54-46-mozilla-central/firefox-41.0a1.en-US.win32.txt build_type: nightly build_url: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015/05/2015-05-27-13-54-46-mozilla-central/firefox-41.0a1.en-US.win32.zip changeset: ff2e07228041 pushlog_url: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c6bbf8f1b02b&tochange=ff2e07228041 repo: mozilla-central repository: https://hg.mozilla.org/mozilla-central
|
||
Comment 6•9 years ago
|
||
[Tracking Requested - why for this release]: [Tracking Requested - why for this release]: Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=dc683817edec&tochange=1921222e708e Suspect: 0431f570160d Michael Layzell — Bug 1162952 - Return true from document.queryCommandEnabled('cut'/'copy') when in privileged or user-initiated code. r=ehsan
|
||
Comment 7•9 years ago
|
||
I believe that this is intended behavior. On firefox startup under the default configuration, your focus is in the about:home textbox. about:home is an HTML page, and in HTML pages we now need to always enable the cut and copy commands. Thus, they are enabled on startup. The cut and copy commands should always be enabled in HTML pages, as it is now possible to intercept and handle the cut and copy events in web pages. As these events should be possible to fire whether or not there is a selection (and it is possible to copy values to the clipboard whether or not there is a selection), we must keep these commands enabled in case they are intercepted and handled by custom content javascript code. I'm ni-ing ehsan to confirm that I'm correct on this.
(In reply to Michael Layzell [:mystor] from comment #7) > I believe that this is intended behavior. On firefox startup under the > default configuration, your focus is in the about:home textbox. about:home > is an HTML page, and in HTML pages we now need to always enable the cut and > copy commands. Thus, they are enabled on startup. > > The cut and copy commands should always be enabled in HTML pages, as it is > now possible to intercept and handle the cut and copy events in web pages. > As these events should be possible to fire whether or not there is a > selection (and it is possible to copy values to the clipboard whether or not > there is a selection), we must keep these commands enabled in case they are > intercepted and handled by custom content javascript code. > > I'm ni-ing ehsan to confirm that I'm correct on this. I have confirmed the cut and copy status (NOT all view) on the latest Nightly. If you are correct, should fix [disabled] items? [Enabled] about:accounts about:app-manager about:buildconfig about:cache about:crashes about:credits about:healthreport about:home about:license about:logo about:memory about:mozilla about:networking about:performance about:plugins about:privatebrowsing about:rights about:robots about:serviceworkers about:sessionrestore about:support about:sync-log about:telemetry about:webrtc about:welcomeback Developer Tools: searchbar in Rules, Computed Developer Tools: code panes [Disabled] urlbar in nav-bar searchbar in nav-bar searchbar in Bookmarks sidebar about:addons about:config about:newtab about:preferences#general Home Page about:preferences#search Keyword about:preferences#content Exceptions -> Address of website about:preferences#applications Searchbar about:preferences#privacy Exceptions -> Address of website about:preferences#privacy Show Cookies -> Searchbar about:preferences#security Exceptions -> Address of website about:preferences#advanced Network -> Connection Settings about:permissions about:sync-tabs Developer Tools: searchbar in Inspector Developer Tools: Web Console Developer Tools: searchbar in Debugger Developer Tools: searchbar in Network Monitor Bookmark Library History Library
|
||
Comment 9•9 years ago
|
||
This is intended. Neil Deakin asked us to disable this behavior for XUL documents which is why these entries are disabled in the pages you listed above. There is no work remaining to be done here.
Based on comment 9, I do not feel the need to track this for FF41 and FF42.
|
||
Comment 11•9 years ago
|
||
(In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!) from comment #9) > This is intended. Neil Deakin asked us to disable this behavior for XUL > documents which is why these entries are disabled in the pages you listed > above. There is no work remaining to be done here. Why? Users don't know HTML or xul documents. Especially about:home and about:newtab. It's just strange behavior for users. That all.
|
|
||
Comment 12•9 years ago
|
||
(In reply to magicp from comment #11) > (In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, > needinfo? me!) from comment #9) > > This is intended. Neil Deakin asked us to disable this behavior for XUL > > documents which is why these entries are disabled in the pages you listed > > above. There is no work remaining to be done here. > > Why? Users don't know HTML or xul documents. Especially about:home and > about:newtab. It's just strange behavior for users. That all. Asking Neil. I have no strong opinions on this.
|
||
Comment 13•9 years ago
|
||
The issue is that HTML doesn't provide a means of indicating to the UI that the clipboard commands should be enabled. Rather than provide this, bug 1159490 just made them enabled all the time. Chrome does something similar. XUL does provide such a means so it should be used to ensure the UI is correct (as Chrome also does for the browser UI) We could do something similar though and use the right UI for about pages or chrome: files or something. Note that in my opinion, the html behaviour is broken and bug 1159490 is invalid.
|
|
||
Comment 16•6 years ago
|
||
The problem appears only on Firefox. Edge and Chrome is as expected, |copy||cut| contextmenu is disabled if nothing is selected in input field. This bug means that Firefox is a liar. I think it need to re-consider this bug.
|
Assignee | |
Updated•6 years ago
|
|
||
Comment 18•5 years ago
|
||
Nika,
As part of the load all XUL as XHTML work (bug 1550801), I've been running into issues with a number of copy/paste tests since we can no longer only filter XUL docs to allow if copy/paste in enabled.
I think we have two options going forward:
- Change the checks that use to be IsHTMLOrXHTML to also check if the document is chrome privileged. If it is chrome privilege than we let it control copy/paste enable/disable.
or
- We could get rid of the behavior that allows XUL to control copy/paste so it behaves like XHTML.
I lean towards #1, but wanted to get your feedback before I start updating tests.
|
|
||
Comment 19•5 years ago
|
||
(In reply to Brendan Dahl [:bdahl] from comment #18)
I lean towards #1, but wanted to get your feedback before I start updating tests.
I don't have any super strong feelings about this, so I would be OK with either approach. Given that our chrome HTML/XHTML is supposed to act like native UI, it probably makes sense to preserve the behaviour rather than returning to web behaviour.
I wonder if it would make sense to have this as a chrome-only attribute which can be set on the document or similar, rather than using the document's principal.
ni? :ehsan who helped me make the decision ~4 years ago when it first came up.
|
|
||
Comment 20•5 years ago
|
||
See comment 13. The reason why this was previously tied to the document being XUL was that XUL provides the mechanism for the document to tell the UA whether copy/cut commands should be enabled or not through the XUL command controller infrastructure (hopefully I'm using the right terminology to refer to it... the code in dom/commandhandler is what I'm referring to). That infrastructure just doesn't exist in HTML at all.
Note that this has nothing to do with chrome privileges and is available to non-privileged XUL documents as well. So I don't think either of the ideas in (1) in comment 18 are exactly the equivalent of what we used to have, and (2) is non-desirable.
So Brendan, in response to your question, I'll ask, what is the plan for replacing the command controller infrastructure in HTML? Based on the answer to that question, this mechanism should be tied to where that replacement is available. I hope that makes sense. :-)
|
|
||
Comment 21•5 years ago
|
||
The command/broadcaster infrastructure has already been moved out of XUL and is supported in chrome privileged HTML too (bug 1486888). Whether we want to keep that around in the long term is a different question, but we do currently have the ability to control enable/disable in HTML.
|
|
||
Comment 22•5 years ago
|
||
(In reply to Brendan Dahl [:bdahl] from comment #21)
The command/broadcaster infrastructure has already been moved out of XUL and is supported in chrome privileged HTML too (bug 1486888).
Hmm, based on https://searchfox.org/mozilla-central/rev/153172de0c5bfca31ef861bd8fc0995f44cada6a/dom/xul/nsXULElement.cpp#691 this isn't entirely true... There is nothing in this code that looks at privileges that I can see. Where do we check to make sure this is restricted to chrome-privileged documents?
Whether we want to keep that around in the long term is a different question, but we do currently have the ability to control enable/disable in HTML.
Well, by HTML I meant HTML in the sense of the HTML spec. HTML + elements from the XUL namespace isn't really HTML. :-) (Not trying to nitpick over terminology of course, just clarifying what I meant... I specifically did not mean any document where IsHTMLOrXHTML() returns true for.)
|
|
||
Comment 23•5 years ago
|
||
(In reply to :Ehsan Akhgari from comment #22)
Hmm, based on https://searchfox.org/mozilla-central/rev/153172de0c5bfca31ef861bd8fc0995f44cada6a/dom/xul/nsXULElement.cpp#691 this isn't entirely true... There is nothing in this code that looks at privileges that I can see. Where do we check to make sure this is restricted to chrome-privileged documents?
XUL elements can't be created (from content) in content privileged pages.
Well, by HTML I meant HTML in the sense of the HTML spec. HTML + elements from the XUL namespace isn't really HTML. :-) (Not trying to nitpick over terminology of course, just clarifying what I meant... I specifically did not mean any document where
IsHTMLOrXHTML()returns true for.)
Yes, sorry to be clear I mean chrome privileged HTML.
|
|
||
Comment 24•5 years ago
|
||
(In reply to Brendan Dahl [:bdahl] from comment #23)
(In reply to :Ehsan Akhgari from comment #22)
Hmm, based on https://searchfox.org/mozilla-central/rev/153172de0c5bfca31ef861bd8fc0995f44cada6a/dom/xul/nsXULElement.cpp#691 this isn't entirely true... There is nothing in this code that looks at privileges that I can see. Where do we check to make sure this is restricted to chrome-privileged documents?
XUL elements can't be created (from content) in content privileged pages.
They can with the allowXULXBL permission, no? The actual condition isn't based on permissions at all: https://searchfox.org/mozilla-central/rev/fe7dbedf223c0fc4b37d5bd72293438dfbca6cec/dom/xul/nsXULElement.cpp#260.
Well, by HTML I meant HTML in the sense of the HTML spec. HTML + elements from the XUL namespace isn't really HTML. :-) (Not trying to nitpick over terminology of course, just clarifying what I meant... I specifically did not mean any document where
IsHTMLOrXHTML()returns true for.)Yes, sorry to be clear I mean chrome privileged HTML.
To the best of my knowledge, we currently have these two restrictions in Gecko:
- We create XUL elements where we grant the allowXULXBL permission, see the link above.
- We allow loading XBL bindings from system privileged or chrome:// URI stylesheets unless the document has allowXULXBL permission.
(Note that Document::AllowXULXBL() returns true for chrome-privileged documents.)
Since what bug 1486888 implemented needs to work where XUL elements can work, I think that feature needs to be gated on Document::AllowXULXBL() checks. I think that would make things work exactly the same way as they did in the browser.xul era. Can you please give that a shot and see if the tests you mentioned still fail with the fixes for bug 1550801?
|
|
||
Comment 25•5 years ago
|
||
For the time being, I tried gating on Document::AllowXULXBL instead, but then several other tests start failing (e.g. test_bug1067255.html) since they are plain html mochitests, but they use super powers (allowXULXBL is set to true). I still think it makes sense to gate this on system principal instead. We only really want this special behavior enabled for chrome documents and we don't need to support it in content documents. There is the case of content documents force enabling XUL/XBL, but that's only ever done in crashtests/reftests and some mochitests, so I don't see why we need to support that use case.
If you have another suggestion I'm open to trying other things.
|
|
||
Comment 26•5 years ago
|
||
(In reply to Brendan Dahl [:bdahl] from comment #25)
For the time being, I tried gating on Document::AllowXULXBL instead, but then several other tests start failing (e.g. test_bug1067255.html) since they are plain html mochitests, but they use super powers (allowXULXBL is set to true). I still think it makes sense to gate this on system principal instead. We only really want this special behavior enabled for chrome documents and we don't need to support it in content documents. There is the case of content documents force enabling XUL/XBL, but that's only ever done in crashtests/reftests and some mochitests, so I don't see why we need to support that use case.
If you have another suggestion I'm open to trying other things.
Ugh, sorry, I really thought that would have worked, otherwise wouldn't have made you do the work. I suppose the good shouldn't be the enemy of the perfect here. You're right, since there is no need to support that use case, let's just gate this on the system principal-ness. Does that actually solve the problem at hand?
|
||
Updated•2 years ago
|
Description
•