1589595 - Unable to copy the selected text from disabled input ("Title", "Render Mode", etc) in Page Info. And Unable to copy tree item url in Page Info

Status: VERIFIED FIXED

(Firefox :: Page Info Window, defect)

Description

[Alice0775 White]

[Tracking Requested - why for this release]:

STR:

1. Open any page
2. Open Page Info View
3. Select text > right click on the selected text > Choose "Copy" > Paste to elsewhere
   --- nothing is copied
4. Select a list item in tree view
5. Right click on the selected > Try to Choose "Copy"
   --- the "Copy" is graytext, unable to choose
6. Right click on tree view > Choose "Select All"
7. Right click on the selected > Try to Choose "Copy"
   --- the "Copy" is graytext, unable to choose

Actual Results:
Copy command is completely broken

Expected Results:
Copy command should work as before

Regression Window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=0b26b9b42bd72a8207811df1234d23b29bc6e846&tochange=865a39c9635f1820c4a6ead25d32da3fab14a8be

Regressed by:
865a39c9635f1820c4a6ead25d32da3fab14a8be Brian Grinstead — Bug 1550549 - Add test coverage for a http domain in page info security tab r=Gijs
b02ae35438c98031901231f4730c729f00def66a Brian Grinstead — Bug 1550549 - Convert XUL textboxes to HTML inputs in pageInfo.xul r=fluent-reviewers,Gijs,flod

Comment 1

[Brian Grinstead [:bgrins]]

(In reply to Alice0775 White from comment #0)

[Tracking Requested - why for this release]:

STR:

1. Open any page
2. Open Page Info View
3. Select text > right click on the selected text > Choose "Copy" > Paste to elsewhere
   --- nothing is copied

This seems to work for me locally (at least grabbing the value of fields like "Title", "Address", "Type", etc). Does this reproduce always or only sometimes or depending on how you select the text?

4. Select a list item in tree view
5. Right click on the selected > Try to Choose "Copy"
   --- the "Copy" is graytext, unable to choose
6. Right click on tree view > Choose "Select All"
7. Right click on the selected > Try to Choose "Copy"
   --- the "Copy" is graytext, unable to choose

I can reproduce these steps even on release (Copy is disabled). Are you sure this is a related regression? I'd be surprised if textbox changes would have affected the context menu for trees.

Comment 2

[Alice0775 White]

Attachment: screenshot

(In reply to Brian Grinstead [:bgrins] from comment #1)

This seems to work for me locally (at least grabbing the value of fields like "Title", "Address", "Type", etc). Does this reproduce always or only sometimes or depending
on how you select the text?

Reproducible : always

Steps to reproduce:

1. Selected by double clicking on "Standards compliance mode" or selected by mouse dragging. See attached screenshot.
   (* Not only "Standards compliance mode" but also "19.29 KB (19,752 bytes)" etc...)
2. And right click and then choose "Copy" of context menu
3. Paste to Notepad.exe or SearchBar of Nightly

Actual Results:
Nothing copied (i.e, previously copied text is pasted)

Note:
(Ctrl+C works as expced)

Comment 3

[Brian Grinstead [:bgrins]]

Updating the title to clarify based on Comment 2, and to not include the tree issue.

Comment 4

[Alice0775 White]

(In reply to Brian Grinstead [:bgrins] from comment #1)

4. Select a list item in tree view
5. Right click on the selected > Try to Choose "Copy"
   --- the "Copy" is graytext, unable to choose
6. Right click on tree view > Choose "Select All"
7. Right click on the selected > Try to Choose "Copy"
   --- the "Copy" is graytext, unable to choose

I can reproduce these steps even on release (Copy is disabled). Are you sure this is a related regression? I'd be surprised if textbox changes would have affected the context menu for trees.

I cannot reproduce the issue above on Firefox70.0RC. (i.e, works as expected on 70.0RC).
So this is definitely regressed in 71.0a1. And regressed by same Bug 1550549.

(In reply to Brian Grinstead [:bgrins] from comment #3)

Updating the title to clarify based on Comment 2, and to not include the tree issue.

summary re-changed

Comment 5

[Alice0775 White]

Attachment: screenshot 71.0a1 vs 70.0RC, context menu of tree item

Comment 6

[Pascal Chevrel:pascalc]

Tracking for 71 (that means a beta uplift as today is merge day)

Comment 7

[Pascal Chevrel:pascalc]

Brian, can you give us an update on this regression wrt to our 71 release? Thanks

Comment 8

[alexander :surkov (:asurkov)]

(In reply to Alice0775 White from comment #2)

Reproducible : always

Steps to reproduce:

1. Selected by double clicking on "Standards compliance mode" or selected by mouse dragging. See attached screenshot.
   (* Not only "Standards compliance mode" but also "19.29 KB (19,752 bytes)" etc...)
2. And right click and then choose "Copy" of context menu
3. Paste to Notepad.exe or SearchBar of Nightly

Actual Results:
Nothing copied (i.e, previously copied text is pasted)

I can reproduce it. Not clear on what may be broken here though. pageInfo cmd_copy takes care about trees only [1], so I assume a standard mechanism should be involved here, something like [2]. I wonder if this mechanism is possibly XUL oriented and thus fails to process html:input (that was a primary change of the regressing bug [3]). I'm not familiar with the code, but perhaps somebody can put some light on it?

Emilio, is it something you ever dealt with by any chance?

[1] https://searchfox.org/mozilla-central/source/browser/base/content/pageinfo/pageInfo.js#1207
[2] https://searchfox.org/mozilla-central/source/dom/base/nsGlobalWindowCommands.cpp#527
[3] https://hg.mozilla.org/mozilla-central/rev/b02ae35438c9

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Not really, I'm not familiar with the semantics of XUL commands. But I can tell you that we're not hitting that code. So I'd try to put a <textbox> back, see if we reach it, and why...

Let me know if I can help with something like that.

Comment 10

[Brian Grinstead [:bgrins]]

Do we need the custom cmd_copy (https://searchfox.org/mozilla-central/rev/088e2cf29c59d733d57af43903eb0267dbf72e2a/browser/base/content/pageinfo/pageInfo.xul#51) and that doCopy function (https://searchfox.org/mozilla-central/rev/088e2cf29c59d733d57af43903eb0267dbf72e2a/browser/base/content/pageinfo/pageInfo.js#1200)? I guess this is a limitation of xul:tree and whatever selection it exposes, but I suspect that if we removed that custom <command> element it would at least fix the <input> copying. Alex can you confirm that?

Comment 11

[Brian Grinstead [:bgrins]]

Oh I wonder if the [command=cmd_copy] we call for the <html:input> at https://searchfox.org/mozilla-central/rev/088e2cf29c59d733d57af43903eb0267dbf72e2a/toolkit/content/editMenuOverlay.js#119 ends up triggering the custom cmd_copy / doCopy for pageInfo on Windows (which then sees it's not a <tree> and doesn't do anything).

If that's the case, then perhaps doing a find/replace from cmd_copy to cmd_copytree in pageInfo.xul will fix both issues.

Comment 12

[Brian Grinstead [:bgrins]]

Without having reproduced this locally yet, I've done a try push with the idea from Comment 11: https://treeherder.mozilla.org/#/jobs?repo=try&revision=db0d358a94eed4e3bb0a5edb75eed2c9f9f72e10

Comment 13

[Brian Grinstead [:bgrins]]

Could you please test with the build at https://treeherder.mozilla.org/#/jobs?repo=try&revision=db0d358a94eed4e3bb0a5edb75eed2c9f9f72e10&selectedJob=272637094 if you have a chance and let me know if that fixes the issues?

Comment 14

[Alice0775 White]

(In reply to Brian Grinstead [:bgrins] from comment #13)

Could you please test with the build at https://treeherder.mozilla.org/#/jobs?repo=try&revision=db0d358a94eed4e3bb0a5edb75eed2c9f9f72e10&selectedJob=272637094 if you have a chance and let me know if that fixes the issues?

Yes, the try build fixes the problem.

Comment 15

[Brian Grinstead [:bgrins]]

Attachment: Bug 1589595 - Replace cmd_copy with cmd_copy_tree in page info so the tree copy handler doesn't override the input field copy handler

Previously when uing <xul:textbox> the context menu was handled by the moz-input-box and not cmd_copy.
Now that these are <html:input> they rely on the cmd_copy by name through a menuitem created in editmenuoverlay
at https://searchfox.org/mozilla-central/rev/088e2cf29c59d733d57af43903eb0267dbf72e2a/toolkit/content/editMenuOverlay.js#119.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c2ac5c82a898
Replace cmd_copy with cmd_copy_tree in page info so the tree copy handler doesn't override the input field copy handler r=Gijs

Comment 17

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/c2ac5c82a898

Comment 18

[Pascal Chevrel:pascalc]

Brian, could you request an uplift to beta please? Thanks

Comment 19

[Brian Grinstead [:bgrins]]

Could you please verify the fix on Nightly?

Comment 20

[Alice0775 White]

I can reproduce the issue on Nightly72.0a1(BuildID 20191024214023).
And I verified fix on Nightly72.0a1(BuildID 20191028094851).

Comment 21

[Brian Grinstead [:bgrins]]

Comment on attachment 9103788 [details]
Bug 1589595 - Replace cmd_copy with cmd_copy_tree in page info so the tree copy handler doesn't override the input field copy handler

Beta/Release Uplift Approval Request

• User impact if declined: Unable to copy selected text from fields in the Page Info dialog.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: STR in Comment 0 (https://bugzilla.mozilla.org/show_bug.cgi?id=1589595#c0)
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is only a change to the Page Info window so won't affect the primary browsing UI. Also, it's a small commit (ID changes for a couple of elements in the page) and it'd be easy to roll back if bugs are reported.
• String changes made/needed:

Comment 22

[Pascal Chevrel:pascalc]

Comment on attachment 9103788 [details]
Bug 1589595 - Replace cmd_copy with cmd_copy_tree in page info so the tree copy handler doesn't override the input field copy handler

71 regression, low risk patch verified on Nightly by the reporter, uplift approved for 71 beta 6, thanks.

Comment 23

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3cf69f0f8361

Comment 24

[Timea Cernea [:tbabos][inactive]]

Reproduced on affected Firefox Beta 5 on Windows 10.
Verified-Fixed on latest Beta 71.0b6 (Build ID: 20191101011821) on Windows 10, MacOS 10.15 and Ubuntu 18.04.

Comment 25

[alexander :surkov (:asurkov)]

nice to see the bug fixed! and thus cancelling my ni? from myself