674771 - Add a "Paste & Go" entry in the URL bar context menu

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, enhancement, P1)

Description

[Mounir Lamouri (:mounir)]

This feature that is in Firefox Desktop would be really useful in Firefox Mobile! :)

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

I agree. This would be nice feature. I have had to "paste and press 'Go'" lately and it was slightly painful.

Comment 2

[Martijn Wargers (dead)]

It's extra painful, because there is no "Go" button at the right side of the url bar.

Comment 3

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

Mark, let me know how that looks like...

Comment 4

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

With the real patch, it's even better!

Comment 5

[Mounir Lamouri (:mounir)]

Comment on attachment 552418 [details] [diff] [review]
Patch v1

Actually, this patch still need some love... Will ask another review later.

Comment 6

[Mounir Lamouri (:mounir)]

Attachment: Patch v2

Two mistakes were in the previous patch:
1. I did some clean-up just before attaching and realized I removed the line defining |target| in pasteAndGo function...
2. The "Paste and Go" context menu item was being shown even for non-url textboxes.

Mark, are they tests for Fennec context menu? I wasn't able to find them :(

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 552624 [details] [diff] [review]
Patch v2

>diff --git a/mobile/chrome/content/bindings.xml b/mobile/chrome/content/bindings.xml

>-          if (hasData && (!aTextbox.readOnly || aIgnoreReadOnly))
>+          if (hasData && (!aTextbox.readOnly || aIgnoreReadOnly)) {
>             json.types.push("paste");
>+            if (aTextbox.type == "url") {
>+              json.types.push("paste-n-go");
>+            }
>+          }

Let's be slightly more general here. Since we are testing pasting into a URL textbox, let's use "paste-url" instead of "paste-n-go"

>diff --git a/mobile/chrome/content/browser.xul b/mobile/chrome/content/browser.xul

>+          <richlistitem class="context-command" id="context-paste-n-go" type="paste-n-go" onclick="ContextCommands.pasteAndGo();">
>+            <label value="&pasteandgo.label;"/>
>+          </richlistitem>

Make the "paste-url" change here and we try to use a &pasteAndGo;" style for entities

>diff --git a/mobile/locales/en-US/chrome/browser.dtd b/mobile/locales/en-US/chrome/browser.dtd

> <!ENTITY paste.label           "Paste">
>+<!ENTITY pasteandgo.label      "Paste and Go">

Desktop uses "Paste & Go" so let's copy that

r+ with nits fixed

Comment 8

[Mounir Lamouri (:mounir)]

(In reply to Mark Finkle (:mfinkle) from comment #7)
> > <!ENTITY paste.label           "Paste">
> >+<!ENTITY pasteandgo.label      "Paste and Go">
> 
> Desktop uses "Paste & Go" so let's copy that

I forgot to mention this but I didn't use "Paste & Go" because it seems to be invalid inside the DTD. I tried "\&" but it didn't work either. I wonder what syntax would work.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Mounir Lamouri (:volkmar) from comment #8)
> (In reply to Mark Finkle (:mfinkle) from comment #7)
> > > <!ENTITY paste.label           "Paste">
> > >+<!ENTITY pasteandgo.label      "Paste and Go">
> > 
> > Desktop uses "Paste & Go" so let's copy that
> 
> I forgot to mention this but I didn't use "Paste & Go" because it seems to
> be invalid inside the DTD. I tried "\&" but it didn't work either. I wonder
> what syntax would work.

"Paste &amp; Go"

Comment 10

[Mounir Lamouri (:mounir)]

Pushed to inbound with the requested changes.

Thank you for the quick review Mark! :)

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/mozilla-central/rev/9e269bcacf43

Comment 12

[Aaron Train [:aaronmt]]

Verified Fixed
Mozilla/5.0 (Android; Linux armv7l; rv:8.0a1) Gecko/20110815 Firefox/8.0a1 Fennec/8.0a1

Andreaa, can you please create a new Litmus BFT test?

Comment 13

[Andreea Pod]

https://litmus.mozilla.org/show_test.cgi?id=29701 testcase added

Comment 14

[Mounir Lamouri (:mounir)]

(In reply to Andreea Pod from comment #13)
> https://litmus.mozilla.org/show_test.cgi?id=29701 testcase added

Thanks :)

Comment 15

[Andreea Pod]

(In reply to Mounir Lamouri (:volkmar) from comment #14)
> (In reply to Andreea Pod from comment #13)
> > https://litmus.mozilla.org/show_test.cgi?id=29701 testcase added
> 
> Thanks :)

With pleasure and sorry for adding it so late.