Closed
Bug 717772
Opened 13 years ago
Closed 13 years ago
Delay autocomplete of pasted value
Categories
(Firefox :: Address Bar, defect)
Firefox
Address Bar
Tracking
()
RESOLVED
FIXED
Firefox 12
People
(Reporter: mak, Assigned: mak)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
There are security concerns in allowing to inline autocomplete pasted partial urls, since a user may paste a partial url and click enter without noticing the url he is going to is actually different from what he expected.
Assignee | ||
Comment 1•13 years ago
|
||
we may need a custom cmd_paste controller to detect this, similar to the _copyCutController one... I don't have better ideas atm, so any alternative idea is welcome.
Assignee | ||
Comment 2•13 years ago
|
||
I don't have better ideas atm, this works as far as the autocomplete fills up synchronously, that is not always true (if we fail matching on a domain it's likely a subpage and autofill for that happens asynchronously). So it's not yet the solution, it's a possible hook point though, that I would like some feedback or ideas on. Maybe we just care about not autocompleting domain matches and not subpages? in such a case it would be enough.
Otherwise we could maybe store the last pasted value and compare with it in the first textValue setter call. It's still sorta hackish though.
Attachment #588430 -
Flags: feedback?(gavin.sharp)
Comment 3•13 years ago
|
||
How about differentiating between input events that add a single character and those that add multiple characters?
Comment 4•13 years ago
|
||
Comment on attachment 588430 [details] [diff] [review]
hack
>diff --git a/browser/base/content/urlbarBindings.xml b/browser/base/content/urlbarBindings.xml
>+ <field name="_pasteController"><![CDATA[
>+ isCommandEnabled: function(aCommand) {
>+ return this.urlbar.inputField.editor.canPaste(Ci.nsIClipboard.kGlobalClipboard);
The nsPasteCommand::isCommandEnabled implementation seems to also check editor.isSelectionEditable, I wonder whether that's necessary?
This doesn't really seem like a hack to me. It should be reliable, right? It would be nice to fix this problem for all autocomplete widgets, though (wait longer before starting the autocomplete searches triggered by pastes).
Attachment #588430 -
Flags: feedback?(gavin.sharp) → feedback+
Assignee | ||
Comment 5•13 years ago
|
||
would something like this be fine? It delays autocomplete on paste by 1 second.
I should still make a test.
Attachment #588888 -
Flags: feedback?(gavin.sharp)
Assignee | ||
Comment 6•13 years ago
|
||
I tried making a chrome mochitest witha type=autocomplete textbox, but the controller complains inputField.editor is null, I wonder if there's a privileges problem, the scope looks correct.
Assignee | ||
Comment 7•13 years ago
|
||
now it works, moving on.
Assignee | ||
Comment 8•13 years ago
|
||
There are still some things to define:
- if the idl changes may be fine
- if the delay should be configurable and how (pref/attribute)
Attachment #588888 -
Attachment is obsolete: true
Attachment #588888 -
Flags: feedback?(gavin.sharp)
Attachment #589169 -
Flags: review?(gavin.sharp)
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Assignee | ||
Comment 9•13 years ago
|
||
Comment on attachment 589169 [details] [diff] [review]
patch v1.0
new patch coming, according to irc discussion with Gavin.
Attachment #589169 -
Attachment is obsolete: true
Attachment #589169 -
Flags: review?(gavin.sharp)
Assignee | ||
Updated•13 years ago
|
Attachment #588430 -
Attachment is obsolete: true
Assignee | ||
Comment 10•13 years ago
|
||
Attachment #589505 -
Flags: review?(gavin.sharp)
Assignee | ||
Updated•13 years ago
|
Flags: in-testsuite+
Summary: Pasted partial urls should not be inline autocompleted → Delay autcomplete of pastes value
Assignee | ||
Updated•13 years ago
|
Summary: Delay autcomplete of pastes value → Delay autocomplete of pasted value
Comment 11•13 years ago
|
||
Comment on attachment 589505 [details] [diff] [review]
patch v2.0
>diff --git a/toolkit/content/widgets/autocomplete.xml b/toolkit/content/widgets/autocomplete.xml
>+ <field name="_pasteController"><![CDATA[
>+ _clipboard: Components.interfaces.nsIClipboard.kGlobalClipboard,
"_kGlobalClipboard"? just "_clipboard" is misleading.
Attachment #589505 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 12•13 years ago
|
||
addressed comment
https://hg.mozilla.org/integration/mozilla-inbound/rev/19a5e75b8ed8
Target Milestone: --- → Firefox 12
Comment 13•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 14•10 years ago
|
||
This seems to have regressed; filed bug 1022399.
You need to log in
before you can comment on or make changes to this bug.
Description
•