Last Comment Bug 699130 - Paste is disabled after using it once in Scratchpad
: Paste is disabled after using it once in Scratchpad
Status: RESOLVED FIXED
[scratchpad][fixed-in-fx-team]
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: unspecified
: x86 Linux
: -- normal (vote)
: Firefox 10
Assigned To: Mihai Sucan [:msucan]
:
: J. Ryan Stinnett [:jryans] (use ni?)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-02 10:28 PDT by Alex Lakatos[:AlexLakatos]
Modified: 2011-11-07 07:10 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
proposed fix (8.05 KB, patch)
2011-11-03 10:02 PDT, Mihai Sucan [:msucan]
rcampbell: review+
Details | Diff | Splinter Review
test fixes (8.60 KB, patch)
2011-11-04 11:53 PDT, Mihai Sucan [:msucan]
no flags Details | Diff | Splinter Review

Description Alex Lakatos[:AlexLakatos] 2011-11-02 10:28:21 PDT
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) 2011-11-02 14:49:12 PDT
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] 2011-11-03 06:45:20 PDT
(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] 2011-11-03 10:02:51 PDT
Created attachment 571681 [details] [diff] [review]
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] 2011-11-03 10:19:44 PDT
(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) 2011-11-04 08:39:31 PDT
Comment on attachment 571681 [details] [diff] [review]
proposed fix

looks fine.
Comment 6 Mihai Sucan [:msucan] 2011-11-04 08:43:23 PDT
This patch didn't pass try server runs, yet. *Almost* there.

Thanks for the r+!
Comment 7 Mihai Sucan [:msucan] 2011-11-04 11:53:14 PDT
Created attachment 572035 [details] [diff] [review]
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) 2011-11-04 12:06:14 PDT
https://hg.mozilla.org/integration/fx-team/rev/e0d2a1287194
Comment 9 Rob Campbell [:rc] (:robcee) 2011-11-05 06:16:05 PDT
https://hg.mozilla.org/mozilla-central/rev/e0d2a1287194
Comment 10 Alex Lakatos[:AlexLakatos] 2011-11-07 07:10:05 PST
VERIFIED in Build identifier: Mozilla/5.0 (X11; Linux i686; rv:10.0a1) Gecko/20111107 Firefox/10.0a1

Note You need to log in before you can comment on or make changes to this bug.