Closed Bug 1661008 Opened 1 year ago Closed 1 year ago

Remote settings: search shortcut tiles should be pinned by default

Categories

(Firefox :: Top Sites, defect, P2)

defect
Points:
3

Tracking

()

VERIFIED FIXED
82 Branch
Iteration:
82.1 - Aug 24 - Sep 6
Tracking Status
firefox81 --- verified
firefox82 --- verified

People

(Reporter: dao, Assigned: dao)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

No description provided.
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Iteration: --- → 82.1 - Aug 24 - Sep 6
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4c71bd5d9ca3
Implement pinning of search shortcut tiles from remote settings. r=mikedeboer
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch

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:
Attachment #9173641 - Flags: approval-mozilla-beta?

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

Approved for 81.0b7.

Attachment #9173641 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Blocks: 1663184
Blocks: 1663179

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.

Flags: needinfo?(dao+bmo)

(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.

(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.

Flags: needinfo?(dao+bmo)

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

Status: RESOLVED → VERIFIED
Blocks: 1687952
You need to log in before you can comment on or make changes to this bug.