1667897 - Make restyleSearches more resilient to changes in SERP URLs

Status: RESOLVED DUPLICATE of bug 1667894

(Firefox :: Address Bar, enhancement, P3)

Description

[Harry Twyford]

restyleSearches is brittle to unexpected differences in SERP URLs. It restyles browsing history by extracting the q parameter from a SERP page in history. It then constructs a URL that matches what that SERP's SearchEngine would construct. If the history result matches the constructed URL, we dedupe the former with the latter.

This doesn't reflect how some organic SERP URLs are constructed; notably Google. For example, if I go to google.com and search for "mozart", I get sent to the URL https://www.google.com/search?hl=en&q=mozart. The hl parameter throws off the matching described in the first paragraph and the mozart SERP isn't restyled.

There's a fine line to walk here, since not all query parameters can be ignored. Some might point to image search, or non-first result pages. Those kinds of URLs shouldn't be deduped. This work will probably require per-engine tuning.

Comment 1

[Harry Twyford]

There's also the URL https://www.google.com/search?hl=en&ei=emFyX8WIDoSltQaO4o_IDw&q=mozart&oq=mozart&gs_lcp=CgZwc3ktYWIQAzIKCC4QsQMQQxCTAjIFCAAQsQMyBAgAEEMyBAgAEEMyAggAMgIIADICCAAyBAgAEEMyAggAMgIIADoHCC4QQxCTAjoKCC4QxwEQowIQQzoHCAAQsQMQQzoCCC46CAgAELEDEIMBOggILhCxAxCDAToFCC4QsQNQ71VY-pYBYJCcAWgCcAB4AIABWogB4wOSAQE2mAEAoAEBqgEHZ3dzLXdperABAMABAQ&sclient=psy-ab&ved=0ahUKEwjFrY3_8IzsAhWEUs0KHQ7xA_kQ4dUDCAw&uact=5 in my history. It shows up in the Urlbar when I search for "mozart"; ideally, it would not.

Comment 2

[Harry Twyford]

Another major win here would be ignoring the attribution parameter in the two URLs. That way, both a search from the Urlbar and an organic search would be considered duplicates, as long as the other parameters were the same. This would require removing the check that the two URLs have the same number of parameters and adding an exception for the attribution parameter in the every() check below, like we do for the terms parameter.

Comment 3

[Harry Twyford]

Setting this as 5 points for now since there are a lot of edge cases. It could be split up into smaller per-engine bugs.

Comment 4

[Harry Twyford]

I was about to file a blocking bug to implement the change in comment 2, turning this into an impromtu meta. Then I realized this bug would just become bug 1667894, its own parent! I'm going to close this one, dupe the comments here, and file future blocking bugs against 1667894.