1661008 - Remote settings: search shortcut tiles should be pinned by default

Status: VERIFIED FIXED

(Firefox :: Top Sites, defect, P2)

Comment 1

[Dão Gottwald [:dao]]

Attachment: Bug 1661008 - Implement pinning of search shortcut tiles from remote settings. r=mikedeboer

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c71bd5d9ca3
Implement pinning of search shortcut tiles from remote settings. r=mikedeboer

Comment 3

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/4c71bd5d9ca3

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 9173641 [details]
Bug 1661008 - Implement pinning of search shortcut tiles from remote settings. r=mikedeboer

Beta/Release Uplift Approval Request

• User impact if declined: This is needed for moving top sites to remote settings in Firefox 81, in case we want to have search shortcuts int he remote settings (possible but not given).
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: QA have a test plan for this.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The whole feature is currently disabled by default.
• String changes made/needed:

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9173641 [details]
Bug 1661008 - Implement pinning of search shortcut tiles from remote settings. r=mikedeboer

Approved for 81.0b7.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e764ee50a2da

Comment 7

[Adrian Florinescu [:aflorinescu]]

I've tested the fix with 82.0a1 2020-09-07 and 81.0b7 2020-09-06 on Windows 10 and the tiles are not pinned for the topsites set as search shortcuts. (one tile pinned out of 4 expected) - check attachment.

Adding the user.js for ease of reproducing.

Comment 8

[Adrian Florinescu [:aflorinescu]]

Attachment: beta7-pinned-tiles.jpg

Comment 9

[Adrian Florinescu [:aflorinescu]]

Attachment: user.js

Comment 10

[Adrian Florinescu [:aflorinescu]]

(In reply to Adrian Florinescu [:aflorinescu] from comment #7)

I've tested the fix with 82.0a1 2020-09-07 and 81.0b7 2020-09-06 on Windows 10 and the tiles are not pinned for the topsites set as search shortcuts. (one tile pinned out of 4 expected) - check attachment.

Giving the above a bit more thought, I think there shouldn't be as expected 4 pinned search shortcuts, but only 1 (amazon) since:
google is default engine, so it should be listed as top site and hence not pinned (bug 1663179)
youtube is not a Firefox built in search engine
baidu is a built in search engine, but I'm guessing we wouldn't want it to be available since it's not enabled for the en-US/RO locale/region used in the above test.

I've also redid the test with a zh-cn/CN which has the baidu engine enabled. The baidu topsite will be available in the default topsites and will become pinned and marked with a search shortcut icon as soon as the default engine is changed away from baidu. However, when switching back to baidu as default search engine, the baidu topsite persists as a searchshortcut and remains pinned.

Comment 11

[Dão Gottwald [:dao]]

(In reply to Adrian Florinescu [:aflorinescu] from comment #10)

(In reply to Adrian Florinescu [:aflorinescu] from comment #7)

I've tested the fix with 82.0a1 2020-09-07 and 81.0b7 2020-09-06 on Windows 10 and the tiles are not pinned for the topsites set as search shortcuts. (one tile pinned out of 4 expected) - check attachment.

Giving the above a bit more thought, I think there shouldn't be as expected 4 pinned search shortcuts, but only 1 (amazon) since:
google is default engine, so it should be listed as top site and hence not pinned (bug 1663179)
youtube is not a Firefox built in search engine
baidu is a built in search engine, but I'm guessing we wouldn't want it to be available since it's not enabled for the en-US/RO locale/region used in the above test.

Yeah, that seems correct.

Comment 12

[Adrian Florinescu [:aflorinescu]]

Verified as fixed on Fx81 as part of PI-763.
Reverified it on Fx 82.0a1 20200915092930: Ubuntu 20.04, Windows 10, Mac 10.13.6.

The above verification takes in account the known issue: bug 1663179