Closed Bug 1405693 Opened 2 years ago Closed 2 years ago

Use dev build of React for Activity Stream if browser.newtabpage.activity-stream.debug is true

Categories

(Firefox :: New Tab Page, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
Firefox 58
Tracking Status
firefox58 --- fixed

People

(Reporter: k88hudson, Assigned: k88hudson)

References

Details

Attachments

(1 file)

This will be available for Nightly or unofficial builds only
Assignee: nobody → khudson
Attachment #8915158 - Flags: review?(edilee)
Comment on attachment 8915158 [details]
Bug 1405693 - Use dev build of React for Activity Stream if browser.newtabpage.activity-stream.debug is true

https://reviewboard.mozilla.org/r/186406/#review191586

We can just land this to autoland and update the PR to be the `bin/render`, etc changes.

::: browser/components/newtab/aboutNewTabService.js:20
(Diff revision 2)
>  XPCOMUtils.defineLazyModuleGetter(this, "AboutNewTab",
>                                    "resource:///modules/AboutNewTab.jsm");
>  
>  const LOCAL_NEWTAB_URL = "chrome://browser/content/newtab/newTab.xhtml";
>  
> -const ACTIVITY_STREAM_URL = "resource://activity-stream/data/content/activity-stream.html";
> +const BASE_AS_PATH = "resource://activity-stream/data/content/activity-stream";

nit: unused

::: browser/components/newtab/aboutNewTabService.js:26
(Diff revision 2)
>  
> -const ACTIVITY_STREAM_PRERENDER_URL = "resource://activity-stream/data/content/activity-stream-prerendered.html";
> +const ACTIVITY_STREAM_URLS = {
> +  "": "resource://activity-stream/data/content/activity-stream.html",
> +  "debug": "resource://activity-stream/data/content/activity-stream-debug.html",
> +  "prerender": "resource://activity-stream/data/content/activity-stream-prerendered.html",
> +  "prerenderdebug": "resource://activity-stream/data/content/activity-stream-prerendered-debug.html",

Personally I think the array with implicit keys is cleaner, but this is fine. Probably add a comment for how these should be used and why they're full specs.

::: browser/components/newtab/aboutNewTabService.js:120
(Diff revision 2)
>            }
>          } else if (data === PREF_ACTIVITY_STREAM_PRERENDER_ENABLED) {
>            this._activityStreamPrerender = Services.prefs.getBoolPref(PREF_ACTIVITY_STREAM_PRERENDER_ENABLED);
>            Services.obs.notifyObservers(null, "newtab-url-changed", ABOUT_URL);
> +        } else if (!IS_RELEASE_OR_BETA && data === PREF_ACTIVITY_STREAM_DEBUG) {
> +          this._activityStreamDebug = Services.prefs.getBoolPref(PREF_ACTIVITY_STREAM_DEBUG, false);

Eh. With 3 of these now, seems like we might want to start thinking of cleaning up with XPCOMUtils.defineLazyPreferenceGetter at some point https://searchfox.org/mozilla-central/source/js/xpconnect/loader/XPCOMUtils.jsm#365-386

::: browser/components/newtab/nsIAboutNewTabService.idl:56
(Diff revision 2)
>    void resetNewTabURL();
> +
> +  /**
> +   * Removes all observers if they exist
> +   */
> +  void uninit();

This isn't called by tests now, so no need to expose
Attachment #8915158 - Flags: review?(edilee) → review+
Pushed by khudson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f01862c860c8
Use dev build of React for Activity Stream if browser.newtabpage.activity-stream.debug is true r=Mardak
https://hg.mozilla.org/mozilla-central/rev/f01862c860c8
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Depends on: 1407844
Component: Activity Streams: Newtab → New Tab Page
You need to log in before you can comment on or make changes to this bug.