Closed Bug 1480507 Opened Last year Closed Last year
Add/Edit new Top Search Shortcut
52 bytes, text/x-github-pull-request
|Details | Review|
We would like adding/editing for Top Search Shortcuts. This will involve opening a special menu that gives users the option to add Top Search Shortcuts, as well as persistence for wherever the shortcut ends up being.
After some discussion with Maria, we've decided this feature is pretty critical to shipping so we'd really like to get this in if possible
The design has "Add a Search Shortcut" which I would assume is in the context menu and for the modal header. These strings would need to be localized and uplifted by ~beta 20 in less than 2 weeks. I suppose potentially "Add search engine" could be reused to avoid needing new strings? https://searchfox.org/mozilla-central/rev/0f4d50ff5211e8dd85e119ef683d04b63062ed90/browser/locales/en-US/chrome/browser/search.properties#34-36 flod, how should we approach strings here?
Looks like the latest design uses the existing "Add Search Engine" string: https://mozilla.invisionapp.com/share/U7N6JJGZ4RB#/screens/313446565_Add_Search_Engines_To_Top_Sites Trying to dynamically load search.properties to get the appropriate string is going to be a lot of custom code as the page expects strings to be readily available via `window.gActivityStreamStrings` Given that we prebuild the strings by fetching from l10n-central, we could specially grab the ones from search.properties, e.g., https://hg.mozilla.org/l10n-central/de/file/default/browser/chrome/browser/search.properties#l42 searchAddFoundEngine2=Suchmaschine hinzufügen … and use it like any other activity steram string by including it in https://searchfox.org/mozilla-central/source/browser/components/newtab/prerendered/locales/de/activity-stream-strings.js Or alternatively, get the string into the actual translated file and just taking the existing translation…? https://hg.mozilla.org/l10n-central/de/file/default/browser/chrome/browser/activity-stream/newtab.properties
Touching the actual newtab.properties file is a no-go. There's no visibility on that for localizers. If you really have to expose this feature, the only approach that would make sense is, for Beta, to pick that string and copy it in the .js cache. Then expose the string correctly in 63.
Proposed design for adding search engine shortcuts to Top Sites: https://mozilla.invisionapp.com/share/U7N6JJGZ4RB#/screens/313446565_Add_Search_Engines_To_Top_Sites
Is it intended for "Add Search Engine" to only be for the packaged search providers? Or should it work for any search engine that the user has added from the web?
Good question. I think surfacing user-added search engines in this modal would be a nice thing to consider in the future but for now we can constrain it to the packaged search providers.
We don't have large icons for a lot of these providers in Firefox (only in Fennec), so that's an issue as well. The current icons are 16x16/32x32. The mobile icons are 96x96.
Commits pushed to master at https://github.com/mozilla/activity-stream https://github.com/mozilla/activity-stream/commit/df19f2dba45bb46dc2de4f6bbf68125e218bd7ef Fix Bug 1480507 - Add/Edit new Top Search Shortcut https://github.com/mozilla/activity-stream/commit/22f333e4328caf54535770dc7a4c3cda795f0618 Merge pull request #4317 from rlr/bug1480507/add-shortcut Fix Bug 1480507 - Add/Edit new Top Search Shortcut
I have verified that the issue is no longer reproducible in the latest Nightly (63.0a1, Build ID 20180815225731) on Windows 10, Mac 10.13 and Linux x64. A section context menu option has been added, "Add Search Engine" that controls which search engines are displayed as Top Sites.
I have verified that the "Add Search Engine" section contest menu option is in the latest Beta (62.0b18 Build ID 20180816151750) on Windows 10, Mac 10.13 and Arch Linux x64 and can be used to control how many search shortcuts appear in the Top Sites section.
Component: Activity Streams: Newtab → New Tab Page
You need to log in before you can comment on or make changes to this bug.