699130 - Paste is disabled after using it once in Scratchpad

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Alex Lakatos[:AlexLakatos]]

Edit > Paste is disabled the second time you try to use it.

Steps To Reproduce:
1.Open Scratchpad
2.Copy some text
3.Edit > Paste
4.Edit > Paste

Actual Results:
4.Paste is disabled

Expected Results:
4.The text is pasted a second time

Comment 1

[Rob Campbell [:rc] (:robcee)]

which version? Do you know if this is a regression?

Also, you mean the menu item is disabled, right? The shortcut key still works.

Comment 2

[Alex Lakatos[:AlexLakatos]]

(In reply to Rob Campbell [:rc] (robcee) from comment #1)
> which version? Do you know if this is a regression?
Mentioned this in bug 694767 and forgot to add a Build ID here. Sorry. It happens on Build identifier: Mozilla/5.0 (X11; Linux i686; rv:10.0a1) Gecko/20111102 Firefox/10.0a1
Can't test for a regression, since the Paste menu entry did not work before this build.
> 
> Also, you mean the menu item is disabled, right? The shortcut key still
> works.
Shortcut key and context menu item still works. The top menu entry does not work.

Comment 3

[Mihai Sucan [:msucan]]

Attachment: proposed fix

Proposed fix.

Problem description: there's some confusion in the native code that deals with the Edit menu commands, if there's an element with contentEditable focused or not. It seems to be a change in native behavior unrelated to Orion and Scratchpad.

Even simply selecting text and unselecting text causes the same problem, and focus in Orion does not change in such cases. A fix in Orion would mean to over-zealously call element.focus(). I tried this, for example, for the Paste action implementation and that worked. However, it quickly became obvious that this is a wider problem.

I looked in browser.js and it seems the way to deal with the Edit menu items is to call goUpdateGlobalEditMenuItems(). It seems we missed this in Scratchpad.

Please let me know if further changes are needed. Thank you!

Patch pushed to try:

https://tbpl.mozilla.org/?tree=Try&rev=15c9b37c9814

Comment 4

[Mihai Sucan [:msucan]]

(In reply to Mihai Sucan [:msucan] from comment #3)
> Problem description: there's some confusion in the native code that deals
> with the Edit menu commands, if there's an element with contentEditable
> focused or not. It seems to be a change in native behavior unrelated to
> Orion and Scratchpad.

To detail a bit:

STR:

1. select some text with the keyboard, press ctrl-x.
2. go to Edit and see that Paste is disabled.
3. click inside the editor to force focus. (note that focus is still correct, even without this mouse action, you can still use the keyboard to navigate within the code)
4. now Edit > Paste is enabled.
5. select some text again with the keyboard, then drop the selection by pressing any arrow key without holding Shift down.
6. see Edit > Paste is disabled again.

So "confusion" is probably mostly related to how selection is handled by Orion. During selection and caret navigation Orion does not change element focus. The paste bug fixes that have happened in Orion since the last update are unrelated to the general use of selection and caret navigation. These parts of the code haven't changed since the last update.

browser.js mentions that the Edit commands are not updated unless requested, and we never ask for command updates - hence the fix does this.


(btw, thanks Alex for your report!)

Comment 5

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 571681 [details] [diff] [review]
proposed fix

looks fine.

Comment 6

[Mihai Sucan [:msucan]]

This patch didn't pass try server runs, yet. *Almost* there.

Thanks for the r+!

Comment 7

[Mihai Sucan [:msucan]]

Attachment: test fixes

Updated the patch to fix the test failures on Mac systems.

Green try results:
https://tbpl.mozilla.org/?tree=Try&rev=53c6fae80270

Thanks to Panos and Neil Deakin's help!

Comment 8

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/integration/fx-team/rev/e0d2a1287194

Comment 9

[Rob Campbell [:rc] (:robcee)]

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

Comment 10

[Alex Lakatos[:AlexLakatos]]

VERIFIED in Build identifier: Mozilla/5.0 (X11; Linux i686; rv:10.0a1) Gecko/20111107 Firefox/10.0a1