Generated search shortcuts paste links after being copied instead of the string in newtab

VERIFIED FIXED in Firefox 63

Status

()

defect
P1
normal
VERIFIED FIXED
11 months ago
10 months ago

People

(Reporter: cfogel, Assigned: adw)

Tracking

Trunk
Firefox 64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox61 unaffected, firefox62 wontfix, firefox63 verified, firefox64 verified)

Details

Attachments

(3 attachments)

[Affected versions]:
- 62.0b20, 63.0a1(build id: 20180823220048)

[Affected platforms]:
- win 10x64, ubuntu 16.04LTS, macOS 10.13

[Steps to reproduce]:
1. Launch Firefox;
2. Access the newtab page;
3. Click on the button for google or amazon search;
4. Copy the generated string from the address bar "@google ";
5. Paste it anywhere;

[Expected result]:
- the exact string is pasted

[Actual result]:
- "http://google/" is pasted

[Regression range]:
- will check if it's a regression asap

[Additional notes]:
- attached screen recording with the issue;
- the link for google is not working either;
Copying without the space pastes the string properly.
The same behaviour is found for the other @search shortcuts (ex: amazon, bing, duckduckgo, ebay, yandex, twitter)
Not a regression, introduced with 1482205.
Having trouble reproducing on the latest nightly.  Tried on both Mac and Windows.
Flags: needinfo?(cristian.fogel)
Still reproducible with current nightly 63.0a1 (2018-08-28), checked over win10.

It's really important to copy the space after the string as well, otherwise it will not happen.
ex: [@google ] leads to the issue, while [@google] is fine.
Flags: needinfo?(cristian.fogel)
I can also replicate this on Mac. It doesn't happen if you manually type in "@google " but does if you click through from the search shortcut.
Component: Activity Streams: Newtab → Search
Component: Search → Address Bar
When you type it manually, we end up here in the copy code because this.valueIsTyped is true: https://dxr.mozilla.org/mozilla-central/rev/703546ab6d0cb643028a1ab4fda997b38f38a2e6/browser/base/content/urlbarBindings.xml#1321

But when you click a tile, we end up farther down that method and fix up the value, which is why you end up with a weird pseudo URL.  I'm not sure what the best fix is.  Maybe urlbar.search() should just set valueIsTyped to true.

Is this a high priority for Activity Stream?
Blocks: 1480503
Priority: -- → P3
`urlbar.search` needs to simulate the user's typing. Fire an input event, as we do elsewhere to simulate it.
Assignee: nobody → adw
Status: NEW → ASSIGNED
Priority: P3 → P1
Comment on attachment 9010105 [details]
Bug 1485987 - Make urlbar.search() fire an input event.

Marco Bonardo [::mak] has approved the revision.
Attachment #9010105 - Flags: review+
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9828a37ea75f
Make urlbar.search() fire an input event. r=mak
https://hg.mozilla.org/mozilla-central/rev/9828a37ea75f
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Issue is no longer reproducible on 64.0a1 (2018-09-21) over macOS 10.13.6, Ubuntu 16.04, win10x64.
Status: RESOLVED → VERIFIED
Drew, this bug is a P1, does it make sense to uplift it to 63 or can it ride the trains to 64? Thanks
Flags: needinfo?(adw)
Yes, this seems OK to uplift.  Will request
Flags: needinfo?(adw)
Posted patch Beta/63 patchSplinter Review
Approval Request Comment
[Feature/Bug causing the regression]:
Search shortcut tiles on the new tab page -- bug 1480505 added the blue highlight in the urlbar that this bug is about, and bug 1479806 is the main meta bug for the project

[User impact if declined]:
The original bug (bug 1480505) landed in 62, and this bug landed in 64.  So if declined, users will have to wait for two releases before this bug is fixed.

[Is this code covered by automated tests?]:
Yes

[Has the fix been verified in Nightly?]:
Yes

[Needs manual test from QE? If yes, steps to reproduce]:
See comment 0

[List of other uplifts needed for the feature/fix]:
None

[Is the change risky?]:
Low risk

[Why is the change risky/not risky?]:
The patch mainly touches a single urlbar function, urlbar.search(), which is only used by the search tiles on the newtab page

[String changes made/needed]:
None
Attachment #9011031 - Flags: approval-mozilla-beta?
Comment on attachment 9011031 [details] [diff] [review]
Beta/63 patch

Low risk patch fixing a bug in a new 62 feature, uplift approved for 63 beta 9, thanks.
Attachment #9011031 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Fix verified with 63.0b9 on win10x64, macOS 10.9, Ubuntu 16.04.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.