1433938 - Ensure that Synced Tab result appears above History result for the same item

Status: RESOLVED FIXED

(Firefox :: Address Bar, enhancement, P1)

Description

[Ryan Feeley [:rfeeley]]

Currently when you enter in a fragment of a title or URL of a page that exists in history, and is also open on another device, the History item shows first.

We should probably bump up the prominence of the Synced Tab result, as an open tab is likely more relevant than History.

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

I've been meaning to file this bug myself! P1 assuming it's a simple weight-bumping change, but we can reconsider if it's more involved than that.

Comment 2

[Marco Bonardo [:mak]]

it's not a simple weight bump, because synced tabs come from an alien source that cannot mixed up with frecency. We can decide to present them before or after other results. We currently do that after, moving it before other results would be trivial. Though, I suppose it would be wrong to present this before an adaptive result (that comes from previous user selections), thus what we could do is: adaptive, remote tab, others. This should be easy enough to do.

Comment 3

[Marco Bonardo [:mak]]

Attachment: Bug 1433938 - Move Synced Tab matches above general history matches in the Address Bar.

Puts Remote (Synced) Tab matches before other history results.
Changes deduping algorithm to replace simple history matches with tab matches when the url is the same.
Keeps overriding a Remote Tab with a Local Tab when the url is the same.

Review commit: https://reviewboard.mozilla.org/r/220182/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/220182/

Comment 4

[Marco Bonardo [:mak]]

I still have to check Try.
It's possible this somehow bitrots with the autofill patch. We can delay this in case.

Comment 5

[Marco Bonardo [:mak]]

Comment on attachment 8950924 [details]
Bug 1433938 - Move Synced Tab matches above general history matches in the Address Bar.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220182/diff/1-2/

Comment 6

[Marco Bonardo [:mak]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=92a553cb8e35fc285fa501ef70cfccd6bda436bf

Comment 7

[Drew Willcoxon :adw]

Comment on attachment 8950924 [details]
Bug 1433938 - Move Synced Tab matches above general history matches in the Address Bar.

https://reviewboard.mozilla.org/r/220182/#review226996

::: toolkit/components/places/PlacesUtils.jsm:434
(Diff revision 2)
> +  parseActionUrl(url) {
> +    if (url instanceof Ci.nsIURI)
> +      url = url.spec;
> +    else if (url instanceof URL)
> +      url = url.href;
> +    if (!url.startsWith("moz-action:"))

If you're going to add a MOZ_ACTION_REGEX const/getter, then maybe add one for "moz-action:" that you can use here and above in mozActionURI()?

Actually why not just skip this startsWith() here and let the match() handle this case below?

Comment 8

[Marco Bonardo [:mak]]

I'd just expect startsWith being more efficient than a regex. I didn't verify that though.

Comment 9

[Marco Bonardo [:mak]]

Comment on attachment 8950924 [details]
Bug 1433938 - Move Synced Tab matches above general history matches in the Address Bar.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220182/diff/2-3/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6cfa62837419
Move Synced Tab matches above general history matches in the Address Bar. r=adw

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/6cfa62837419

Comment 12

[Tom Bannister]

Can reproduce this issue in Firefox 59.0.3 (20180427210249) on Windows 10x64.

This issue can no longer be reproduced on Firefox 61.0b1 (20180428110614) on Windows 10x64.

[bugday-20180502]