Closed Bug 1298950 Opened 8 years ago Closed 8 years ago

Support about:home overrides in chrome_url_overrides

Categories

(WebExtensions :: General, defect, P3)

defect

Tracking

(firefox51 wontfix, firefox52 wontfix, firefox53 wontfix, firefox54 fixed)

RESOLVED FIXED
mozilla54
Tracking Status
firefox51 --- wontfix
firefox52 --- wontfix
firefox53 --- wontfix
firefox54 --- fixed
webextensions +

People

(Reporter: andy+bugzilla, Assigned: mattw)

References

(Blocks 1 open bug)

Details

(Whiteboard: [design-decision-approved][triaged])

Attachments

(1 file)

This is a split off of bug 1234150 comment 13. That bug is about allowing developers to override about:newtab. In that comment is was suggested we also allow the override about:home.
Priority: -- → P3
Whiteboard: [design-decision-approved][triaged]
webextensions: --- → +
Does Firefox have a provision for editable dropdown widgets in preferences? I've always felt that the "Restore Default" button in General > Preferences was a bit of an inelegant solution and, with a controlled means for handling overrides, it seems like the most elegant solution would be to remove the "Restore Default" button and replace the text field with an editable "pick an available home page or type/paste your own" dropdown field.
Assignee: nobody → mwein
Blocks: 1311472
Comment on attachment 8836670 [details] Bug 1298950 - Add support for overriding about:home to chrome_url_overrides https://reviewboard.mozilla.org/r/112042/#review113374 ::: browser/components/extensions/ext-url-overrides.js:19 (Diff revision 3) > // Bug 1320736 tracks creating a generic precedence manager for handling > // multiple addons modifying the same properties, and bug 1330494 has been filed > // to track utilizing this manager for chrome_url_overrides. Until those land, > // the edge cases surrounding multiple addons using chrome_url_overrides will > // be ignored and precedence will be first come, first serve. > let overrides = { I think overrides should be a weakmap, or at least its members should be weakmaps. ::: browser/components/extensions/test/browser/browser_ext_url_overrides_home.js:12 (Diff revision 3) > + "resource://gre/modules/Preferences.jsm"); > + > +const HOME_URI_1 = "webext-home-1.html"; > +const HOME_URI_2 = "webext-home-2.html"; > +const HOME_URI_3 = "webext-home-3.html"; > + can you add some tests with both home and newtab?
Comment on attachment 8836670 [details] Bug 1298950 - Add support for overriding about:home to chrome_url_overrides https://reviewboard.mozilla.org/r/112042/#review113374 > I think overrides should be a weakmap, or at least its members should be weakmaps. Could you explain to me how I would do this with WeakMaps? The reason I'm using queues is to maintain the order in which overrides are added so I can update them properly when extensions are uninstalled. > can you add some tests with both home and newtab? Yeah, we should definitely have a test for that :)
Comment on attachment 8836670 [details] Bug 1298950 - Add support for overriding about:home to chrome_url_overrides https://reviewboard.mozilla.org/r/112042/#review113374 > Could you explain to me how I would do this with WeakMaps? The reason I'm using queues is to maintain the order in which overrides are added so I can update them properly when extensions are uninstalled. sigh. weakmap not map.
Comment on attachment 8836670 [details] Bug 1298950 - Add support for overriding about:home to chrome_url_overrides https://reviewboard.mozilla.org/r/112040/#review113448 ::: browser/components/extensions/ext-url-overrides.js:87 (Diff revision 4) > > - overrides.newtab.push({extension, url}); > + for (let page of Object.keys(overrides)) { > + if (manifest.chrome_url_overrides[page]) { > + let relativeURL = manifest.chrome_url_overrides[page]; > + let url = extension.baseURI.resolve(relativeURL); > + overrides[page].push({extension, url}); Can we change this to {extension.id, url}, that way we at least do not hold a hard reference to an extension.
Comment on attachment 8836670 [details] Bug 1298950 - Add support for overriding about:home to chrome_url_overrides https://reviewboard.mozilla.org/r/112042/#review113450
Attachment #8836670 - Flags: review?(mixedpuppy) → review+
Keywords: checkin-needed
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e88f2e7f0ee5 Add support for overriding about:home to chrome_url_overrides r=mixedpuppy
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Attachment #8836670 - Flags: review?(aswan)
Depends on: 1341458
No plan to have dot release for 51. Mark 51 won't fix.
Is this worth uplifting to 53?
Flags: needinfo?(mwein)
(In reply to Julien Cristau [:jcristau] from comment #16) > Is this worth uplifting to 53? No.
Flags: needinfo?(mwein)
I wonder if it would be better to support this Chrome API instead ? https://developer.chrome.com/extensions/settings_override Notably the "startup_pages" and the "homepage" parts. They seem more complete than the one added here.
Flags: needinfo?(mwein)
Flags: needinfo?(mixedpuppy)
Flags: needinfo?(amckay)
We've already got that started, Bug 1301315. chrome_url_overrides allows for overriding pages that are not part of settings_override, so different purpose.
Flags: needinfo?(mixedpuppy)
Essentially just reiterating what Shane said - they are both different APIs that we plan to support to the extend that they make sense in Firefox. Let us know if you have any other questions about this. I'm going to clear Andy's needinfo since I think Shane and/or I can hopefully take it from here.
Flags: needinfo?(amckay)
Flags: needinfo?(mwein)
s/extend/extent
Shane, Matt, I don't understand how the "homepage" field is any different than this bug. The patch in this bug just sets browser.startup.homepage, which is basically the browser homepage setting. So chrome_settings_override.homepage is essentially equivalent to this.
Flags: needinfo?(mwein)
Flags: needinfo?(mixedpuppy)
Okay, now I understand... When this landed, Shane and I weren't aware that Chrome had chrome_settings_overrides for the homepage, so we added support for it here. Bug 1341458 was filed right after this landed and moved support for overriding the homepage to chrome_settings_overrides. Now that that has landed, we need to back this bug out.
Flags: needinfo?(mwein)
Shane, would you mind doing the honor of backing this one out?
It was backed out in bug 1344773
Depends on: 1344773
Flags: needinfo?(mixedpuppy)
Thanks
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: