The default bug view has changed. See this FAQ.

Add "Paste and Go" to the context menu of the URLbar.

VERIFIED FIXED

Status

SeaMonkey
Location Bar
--
enhancement
VERIFIED FIXED
7 years ago
6 years ago

People

(Reporter: Philip Chee, Assigned: Philip Chee)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [parity-fx][parity-IE][parity-opera][parity-chrome])

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

7 years ago
Adapt the following Firefox fixes for SeaMonkey:
[Bug 492544] Add 'Paste and Go' + 'Paste and Search' to context menu on location field + search field.
[Bug 597129] "Paste & Go" or "Paste & Search" trigger two pastes, including to content.
(Assignee)

Comment 1

7 years ago
Created attachment 478752 [details] [diff] [review]
Patch v1.0 proposed fix.
(Assignee)

Updated

7 years ago
Component: UI Design → Location Bar
QA Contact: ui-design → location-bar
Whiteboard: parity-fx → [parity-fx][parity-IE][parity-opera][parity-chrome]
(Assignee)

Comment 2

7 years ago
Comment on attachment 478752 [details] [diff] [review]
Patch v1.0 proposed fix.

># HG changeset patch
># User Philip Chee <philip.chee@gmail.com>
># Date 1285582698 -28800
># Node ID 41f0e3ace61e2120b3cb1b9acdd7a7c197eee58b
># Parent  f4a93ad5efb50d350a875c94f1dfdfe8a9fcbec2
>Bug 599833 Add "Paste and Go" to the context menu of the URLbar.
>
>diff --git a/suite/browser/navigator.css b/suite/browser/navigator.css
>--- a/suite/browser/navigator.css
>+++ b/suite/browser/navigator.css
>@@ -19,16 +19,20 @@ tabbrowser {
> }
> 
> /* ::::: urlbar autocomplete ::::: */
> 
> #urlbar {
>   -moz-binding: url("chrome://navigator/content/urlbarBindings.xml#urlbar");
> }
> 
>+#urlbar > .autocomplete-textbox-container > .textbox-input-box {
>+  -moz-binding: url("chrome://navigator/content/urlbarBindings.xml#input-box-paste");
>+}
>+
> panel[for="urlbar"] {
>   -moz-binding: url("chrome://navigator/content/urlbarBindings.xml#autocomplete-result-popup") !important;
> }
> 
> .autocomplete-search-box {
>   -moz-binding: url("chrome://navigator/content/urlbarBindings.xml#autocomplete-search-box");
> }
> 
>diff --git a/suite/browser/urlbarBindings.xml b/suite/browser/urlbarBindings.xml
>--- a/suite/browser/urlbarBindings.xml
>+++ b/suite/browser/urlbarBindings.xml
>@@ -1,28 +1,35 @@
> <?xml version="1.0"?>
> 
>+<!DOCTYPE bindings [
>+  <!ENTITY % textcontextDTD SYSTEM "chrome://communicator/locale/utilityOverlay.dtd">
>+  %textcontextDTD;
>+]>
>+
> <bindings id="urlbarBindings"
>           xmlns="http://www.mozilla.org/xbl"
>           xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
>           xmlns:xbl="http://www.mozilla.org/xbl">
> 
>   <binding id="urlbar" extends="chrome://global/content/autocomplete.xml#autocomplete">
>     <implementation>
>       <constructor><![CDATA[
>         var pbi = this.mPrefs.QueryInterface(Components.interfaces.nsIPrefBranch2);
>         if (pbi)
>           pbi.addObserver("browser.urlbar", this.mPrefObserver, false);
>-        
>+
>         this.updatePref("browser.urlbar.showPopup");
>         this.updatePref("browser.urlbar.autoFill");
>         this.updatePref("browser.urlbar.showSearch");
>+        this.inputField.controllers.insertControllerAt(0, this._editItemsController);
>       ]]></constructor>
> 
>       <destructor><![CDATA[
>+        this.inputField.controllers.removeController(this._editItemsController);
>         var pbi = this.mPrefs.QueryInterface(Components.interfaces.nsIPrefBranch2);
>         if (pbi)
>           pbi.removeObserver("browser.urlbar", this.mPrefObserver);
>       ]]></destructor>
> 
>       <field name="mPrefs">
>         var svc = Components.classes["@mozilla.org/preferences-service;1"]
>                             .getService(Components.interfaces.nsIPrefService);
>@@ -88,19 +95,47 @@
>                 this.mInputElt.setSelectionRange(entry.length, this.value.length);
>                 this.mDefaultMatchFilled = true;
>               }
>               this.mNeedToComplete = true;
>             }
>           }
>         ]]></body>
>       </method>
>+
>+      <field name="_editItemsController"><![CDATA[
>+        ({
>+          doCommand: function(aCommand) {
>+            switch (aCommand) {
>+              case "cmd_pasteAndGo":
>+                goDoCommand("cmd_paste");
>+                handleURLBarCommand(null, null);
>+            }
>+          },
>+          supportsCommand: function(aCommand) {
>+            switch (aCommand) {
>+              case "cmd_pasteAndGo":
>+                return true;
>+            }
>+            return false;
>+          },
>+          isCommandEnabled: function(aCommand) {
>+            switch (aCommand) {
>+              case "cmd_pasteAndGo":
>+                return document.commandDispatcher
>+                               .getControllerForCommand("cmd_paste")
>+                               .isCommandEnabled("cmd_paste");
>+            }
>+          },
>+          onEvent: function(aEventName) {}
>+        })
>+      ]]></field>
>     </implementation>
>   </binding>
>-  
>+
>   <binding id="autocomplete-result-popup" extends="chrome://global/content/autocomplete.xml#autocomplete-result-popup">
>     <content>
>       <xul:tree anonid="tree" class="autocomplete-tree plain" flex="1">
>         <xul:treecols anonid="treecols">
>           <xul:treecol class="autocomplete-treecol" id="treecolAutoCompleteValue" flex="2"/>
>           <xul:treecol class="autocomplete-treecol" id="treecolAutoCompleteComment" flex="1" hidden="true"/>
>         </xul:treecols>
>         <xul:treechildren anonid="treebody" class="autocomplete-treebody" flex="1"/>
>@@ -483,10 +518,32 @@
>         this.parentNode.selectedIndex = Number(this.getAttribute("engineIndex"));
>       </handler>
> 
>       <handler event="mouseout">
>         this.parentNode.selectedIndex = -1;
>       </handler>
>     </handlers>
>   </binding>
>-  
>+
>+  <binding id="input-box-paste" extends="chrome://global/content/bindings/textbox.xml#input-box">
>+    <content context="_child">
>+      <children/>
>+      <xul:menupopup anonid="input-box-contextmenu"
>+                     class="textbox-contextmenu"
>+                     onpopupshowing="if (document.commandDispatcher.focusedElement != this.parentNode.firstChild)
>+                                       this.parentNode.firstChild.focus();
>+                                     this.parentNode._doPopupItemEnabling(this);"
>+                     oncommand="var cmd = event.originalTarget.getAttribute('cmd'); if(cmd) { this.parentNode.doCommand(cmd); event.stopPropagation(); }">
>+        <xul:menuitem label="&undoCmd.label;" accesskey="&undoCmd.accesskey;" cmd="cmd_undo"/>
>+        <xul:menuseparator/>
>+        <xul:menuitem label="&cutCmd.label;" accesskey="&cutCmd.accesskey;" cmd="cmd_cut"/>
>+        <xul:menuitem label="&copyCmd.label;" accesskey="&copyCmd.accesskey;" cmd="cmd_copy"/>
>+        <xul:menuitem label="&pasteCmd.label;" accesskey="&pasteCmd.accesskey;" cmd="cmd_paste"/>
>+        <xul:menuitem label="&pasteGoCmd.label;" accesskey="&pasteGoCmd.accesskey;" cmd="cmd_pasteAndGo"/>
>+        <xul:menuitem label="&deleteCmd.label;" accesskey="&deleteCmd.accesskey;" cmd="cmd_delete"/>
>+        <xul:menuseparator/>
>+        <xul:menuitem label="&selectAllCmd.label;" accesskey="&selectAllCmd.accesskey;" cmd="cmd_selectAll"/>
>+      </xul:menupopup>
>+    </content>
>+  </binding>
>+
> </bindings>
>diff --git a/suite/locales/en-US/chrome/common/utilityOverlay.dtd b/suite/locales/en-US/chrome/common/utilityOverlay.dtd
>--- a/suite/locales/en-US/chrome/common/utilityOverlay.dtd
>+++ b/suite/locales/en-US/chrome/common/utilityOverlay.dtd
>@@ -31,16 +31,18 @@
> <!ENTITY cutCmd.key							"X">  
> <!ENTITY cutCmd.accesskey					"t"> 
> <!ENTITY copyCmd.label						"Copy">  
> <!ENTITY copyCmd.key						"C">  
> <!ENTITY copyCmd.accesskey					"c"> 
> <!ENTITY pasteCmd.label						"Paste">  
> <!ENTITY pasteCmd.key						"V">  
> <!ENTITY pasteCmd.accesskey					"p"> 
>+<!ENTITY pasteGoCmd.label					"Paste &amp; Go">
>+<!ENTITY pasteGoCmd.accesskey				"G"> 
> <!ENTITY deleteCmd.label					"Delete">  
> <!ENTITY deleteCmd.accesskey				"d"> 
> <!ENTITY selectAllCmd.label					"Select All">  
> <!ENTITY selectAllCmd.key					"A">  
> <!ENTITY selectAllCmd.accesskey				"a"> 
> <!ENTITY preferencesCmd.label				"Preferences…">
> <!ENTITY preferencesCmd.key					"E">  
> <!ENTITY preferencesCmd.accesskey			"e">
Attachment #478752 - Flags: review?(neil)
(Assignee)

Comment 3

7 years ago
Comment on attachment 478752 [details] [diff] [review]
Patch v1.0 proposed fix.

Attempt to add comment one more time!

Differences with the Firefox version of this patch:

1. Instead of generating the menu item dynamically I use an additional derived binding:


> +#urlbar > .autocomplete-textbox-container > .textbox-input-box {
> +  -moz-binding: url("chrome://navigator/content/urlbarBindings.xml#input-box-paste");
> +}

2. Instead of enabling/disabling the menu item via a popup event listener I use a custom controller:

> +        this.inputField.controllers.insertControllerAt(0, this._editItemsController);

> +        this.inputField.controllers.removeController(this._editItemsController);

> +      <field name="_editItemsController"><![CDATA[
> +        ({
> +          doCommand: function(aCommand) {
> +            switch (aCommand) {

I am using a switch here to make it easier to port further patches from Firefox e.g. Bug 531210.


> +              case "cmd_pasteAndGo":
> +                goDoCommand("cmd_paste");
> +                handleURLBarCommand(null, null);

I don't know how to get an event to use as the second parameter of handleURLBarCommand()

> +<!ENTITY pasteGoCmd.label					"Paste &amp; Go">
> +<!ENTITY pasteGoCmd.accesskey				"G">

Firefox UX team prefers an ampersand over "and".

Comment 4

7 years ago
Comment on attachment 478752 [details] [diff] [review]
Patch v1.0 proposed fix.

>+          doCommand: function(aCommand) {
Nit: doCommand after isCommandEnabled

>+                goDoCommand("cmd_paste");
I seem to remember seeing a bug that you have to select all first?

>+                handleURLBarCommand(null, null);
[Rather than calling handleURLBarCommand directly, might be neater to call this._fireEvent("textentered", "pasting");]
Nit: missing break; (other cases use return so not a problem)
(Assignee)

Updated

7 years ago
Attachment #478752 - Flags: review?(neil)
(Assignee)

Comment 5

7 years ago
Created attachment 479734 [details] [diff] [review]
Patch v1.1

> neil@parkwaycc.co.uk      2010-09-28 03:38:29 PDT
> 
> Comment on attachment 478752 [details] [diff] [review]
> Patch v1.0 proposed fix.
> 
>> +          doCommand: function(aCommand) {
> Nit: doCommand after isCommandEnabled
Fixed.

>> +                goDoCommand("cmd_paste");
> I seem to remember seeing a bug that you have to select all first?
I found one Firefox bug that seem relevant:
[NEW] Bug 599793 - Paste & Go and Paste & Search should always replace the entire contents of the textbox.
However there are no patches yet for this bug. So I've decided to unconditionally delete the entire current contents of the textbox before pasting.

>> +                handleURLBarCommand(null, null);
> [Rather than calling handleURLBarCommand directly, might be neater to call
> this._fireEvent("textentered", "pasting");]
Fixed.

> Nit: missing break; (other cases use return so not a problem)
Fixed.
Attachment #478752 - Attachment is obsolete: true
Attachment #479734 - Flags: review?(neil)

Comment 6

7 years ago
Comment on attachment 479734 [details] [diff] [review]
Patch v1.1

>+          _fireEvent: this._fireEvent.bind(this),
Sneaky! I wonder whether you would be better off binding doCommand ;-)

>+                this.editor.selectAll();
>+                this.editor.selection.deleteFromDocument();
>+                goDoCommand("cmd_paste");
Paste already deletes the selection, does it not?
Or you could just set the value to "".

>+<!ENTITY pasteGoCmd.accesskey				"G"> 
Nit: trailing space.

Comment 7

7 years ago
(In reply to comment #6)
>(From update of attachment 479734 [details] [diff] [review])
>>+          _fireEvent: this._fireEvent.bind(this),
>Sneaky! I wonder whether you would be better off binding doCommand ;-)
Or you could be boring and store a reference to the element in the object...
(Assignee)

Comment 8

7 years ago
Created attachment 480415 [details] [diff] [review]
Patch v1.2 bind the whole doCommand.

Bug 599833 Add "Paste and Go" to the context menu of the URLbar.

>>+          _fireEvent: this._fireEvent.bind(this),
> Sneaky! I wonder whether you would be better off binding doCommand ;-)
I bound the whole doCommand rather than hold a reference to the urlbar. It's much simpler to just do:

|doCommand: function(aCommand) { ... }.bind(this);|

>>+                this.editor.selectAll();
>>+                this.editor.selection.deleteFromDocument();
>>+                goDoCommand("cmd_paste");
> Paste already deletes the selection, does it not?
> Or you could just set the value to "".
Fixed using |this.value = "";|

>>+<!ENTITY pasteGoCmd.accesskey				"G"> 
> Nit: trailing space.
Fixed.
Attachment #479734 - Attachment is obsolete: true
Attachment #480415 - Flags: superreview?(neil)
Attachment #480415 - Flags: review?(neil)
Attachment #479734 - Flags: review?(neil)

Updated

7 years ago
Attachment #480415 - Flags: superreview?(neil)
Attachment #480415 - Flags: superreview+
Attachment #480415 - Flags: review?(neil)
Attachment #480415 - Flags: review+
(Assignee)

Comment 9

7 years ago
Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/53261a78051d
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Comment 10

7 years ago
You should not have landed this during a string freeze!

That said, it's done, and most localizers caught up and localized it already. Still, please at least post to m.d.l10n and tell them you did break the string freeze, so those who haven't know to catch up.
(Assignee)

Comment 11

7 years ago
Oops sorry. I checked the Tinderbox page and the topic in #seamonkey, but forgot to check the dates for the L10n freeze.
(Assignee)

Comment 12

7 years ago
Created attachment 482237 [details] [diff] [review]
Patch 2.0 isCommandEnabled does not always return a value

From irc://moznet/seamonkey :
> Ratty: hmm, your isCommandEnabled does not always return a value...
Attachment #482237 - Flags: review?(neil)

Updated

7 years ago
Attachment #482237 - Flags: review?(neil) → review+
(Assignee)

Comment 13

7 years ago
> Created attachment 482237 [details] [diff] [review]
> Patch 2.0 isCommandEnabled does not always return a value

Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/c01b6b5f2b8e
Status: RESOLVED → VERIFIED
OK, so we already fixed bug 599793 on our side here. Should we port bug 601695 (Paste & Go should only appear when clipboard is URL)? That would make some special cases (host names, bookmark keywords etc.) not work with P&G anymore so I thought I'd ask here first. If in doubt we can just open a follow-up.
(Assignee)

Comment 15

7 years ago
My take is that if someone really wants to paste a non url e.g. a keyword search we shouldn't try to prevent them from doing so. Micromanaging this sort of stuff just causes end user confusion e.g. "Why is the Paste&Go option greyed out sometimes?".
(Assignee)

Updated

6 years ago
Blocks: 613199
Depends on: 492544, 597129
(Assignee)

Updated

6 years ago
Blocks: 640427
You need to log in before you can comment on or make changes to this bug.