597129 - Web page can steal paste text. Textbox in web page is changed to pasted text temporarily when execute "Paste & Go" or "Paste & Search" command.

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

[Alice0775 White]

Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100916 Firefox/4.0b7pre ID:20100916041016

Textbox in web page changes to pasted text temporarily when execute "Past & Go" or  "Pase & Search" command

Reproducible: Always

Steps to Reproduce1:
1. Start Minefield with new profile
2. Open URL ( about:home or  http://search.yahoo.com/ etc)
3. Copy valid URL to clipboard
4. Right click in LocationBar and Select "Past & Go" in ContextMenu
5. Watch textbox of contentarea carefully.

Steps to Reproduce2:
1. Start Minefield with new profile
2. Open URL ( about:home or  http://search.yahoo.com/ etc)
3. Copy any text to clipboard
4. Right click in SearchBar and Select "Past & Search" in ContextMenu
5. Watch textbox of contentarea carefully.

Actual Results:
 Textbox in web page changes to pasted text temporarily when execute "Past & Go" or  "Pase & Search" command

Expected Results:
 Should not change textbox of webpage.

Comment 1

[Alice0775 White]

Attachment: testcase

[STR]
1. Start Minefield with new profile
2. Copy valid URL/text to clipboard
3. Open URL ( testcase )
4. Right click in LocationBar/SearchBar and Select "Past & Go"/"Past & Search" in ContextMenu

[Actual]
The testCase page steals pasted text

Comment 2

[Frank Yan (:fryn)]

This is caused by using cmd="cmd_paste", which I guess triggers the paste commend again after we've already pasted and gone. Dolske, any ideas for alternatives? I still don't know why the <observes/> that I used failed to work.

Comment 3

[Justin Dolske [:Dolske]]

Need to either fix this or back out Paste&Go for B7. Will look at this today.

Comment 4

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

Bah. Yeah, the cmd= trick worked for enabling the new menuitem, but I didn't notice other code (the menu popup binding's oncommand) was using it to execute the specified command too. So, we were indeed triggering a paste command twice.

This patch removes that hack, and just explicitly uses a popupshowing handler to enable/disable Paste&Go / Paste&Search in the usual way.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 476981 [details] [diff] [review]
Patch v.1

>diff --git a/browser/base/content/urlbarBindings.xml b/browser/base/content/urlbarBindings.xml

>-          element.setAttribute("cmd", "cmd_paste");

I don't understand how this had any effect at all - the attribute usually used for this is "command", not "cmd", and I don't see any magic with command=cmd inheritance or anything like that.

>+        this._contextMenu.removeEventListener("popupshowing", this, false);

I don't think this is necessary. In fact I'd probably prefer just using a closure rather than passing "this" as the handler (makes retrieving the element simpler too).

>+      <field name="_contextMenu"></field>

>+            case "popupshowing":
>+              var items = this._contextMenu.getElementsByAttribute("anonid", "paste-and-go");
>+              if (!items)
>+                return;
>+              var pasteAndGo = items[0];

(Not relevant if you follow my other suggestion, but does querySelector("[anonid=paste-and-go]") work for this?)

Comment 6

[Justin Dolske [:Dolske]]

Attachment: Patch v.2

Comment 7

[Justin Dolske [:Dolske]]

[As noted on IRC, the |cmd| magic happens in toolkit/content/widgets/textbox.xml's _setMenuItemVisibility().]

Comment 8

[Justin Dolske [:Dolske]]

Pushed http://hg.mozilla.org/mozilla-central/rev/2c20189106c0

Thanks for catching this, great find!