1399696 - TypeError: header is null | contentSearchUI.js

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect)

Description

[Ted Campbell [:tcampbell]]

In Nightly 20170913220121, I get TypeErrors when opening debug console on New Tab page.

STR:
  - Remove search from new tab page
  - Reload / Open a new new tab page
  - Open debug console

Expected:
  - Clean console

Actual:
  - TypeError: header is null | contentSearchUI.js:611:5 (_updateDefaultEngineHeader)

Comment 1

[Ed Lee :Mardak]

I can't seem to reproduce. I can set a breakpoint for when the search box is shown, but it never calls into contentSearchUI for me when I remove the search box.

If you turn on "Persist Logs", what do you see when reloading the page when you have search off? Should be one of

jar:file:…[e-mail]!/chrome/content/data/content/activity-stream-prerendered.html
jar:file:…[e-mail]!/chrome/content/data/content/activity-stream.html

Comment 2

[Ted Campbell [:tcampbell]]

I see: [e-mail]!/chrome/content/data/content/activity-stream-prerendered.html

My new tab page has everything disabled. I'm on Win10 64-bit FF. This only started after last nightly update within 24hr.

Comment 3

[Ed Lee :Mardak]

Ah, did you disable things through the in-content preferences? It should have toggled browser.newtabpage.activity-stream.prerender to false automatically, but maybe something is broken.

Comment 4

[Ted Campbell [:tcampbell]]

Correct. I'm using in-content prefs to toggle. It happens on Linux as well for me. If I add search, the error goes away, if I remove it comes back.

The browser.newtabpage.activity-stream.prerender is false in about:config and still uses prerended page.

Comment 5

[Kate Hudson :k88hudson]

Ah, looks like the prerender pref isn't getting properly read at initialization in aboutNewTabService

Comment 6

[Kate Hudson :k88hudson]

Attachment: Bug 1399696 - aboutNewTabService should initialize with the correct _activityStreamPrerender value

Review commit: https://reviewboard.mozilla.org/r/180300/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180300/

Comment 7

[Ed Lee :Mardak]

Comment on attachment 8908632 [details]
Bug 1399696 - aboutNewTabService should initialize with the correct _activityStreamPrerender value

https://reviewboard.mozilla.org/r/180300/#review185458

Maybe just initialize in the constructor just before calling `this.toggleActivityStream();` ?

::: browser/components/newtab/aboutNewTabService.js:139
(Diff revision 1)
>      if (stateEnabled) {
>        this._activityStreamEnabled = true;
>      } else {
>        this._activityStreamEnabled = false;
>      }
> +    this._activityStreamPrerender = Services.prefs.getBoolPref(PREF_ACTIVITY_STREAM_PRERENDER_ENABLED);

The constructor does call `toggleActivityStream` but we should already be updating this state when the pref actually changes. Any reason not to just initialize correctly from the constructor?

Comment 8

[Kate Hudson :k88hudson]

Comment on attachment 8908632 [details]
Bug 1399696 - aboutNewTabService should initialize with the correct _activityStreamPrerender value

https://reviewboard.mozilla.org/r/180300/#review185458

> The constructor does call `toggleActivityStream` but we should already be updating this state when the pref actually changes. Any reason not to just initialize correctly from the constructor?

The pref can start off being different on browser initialization (i.e. from previous changes to the pref), which will never trigger the observer

Comment 9

[Kate Hudson :k88hudson]

Comment on attachment 8908632 [details]
Bug 1399696 - aboutNewTabService should initialize with the correct _activityStreamPrerender value

https://reviewboard.mozilla.org/r/180300/#review185458

To be honest I think specifically initializing values in the constructor would make more sense than adding to toggleActivityStream, but I didn't want to do any refactoring considering we're pretty close to the cut-off

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2ad95439eef6
aboutNewTabService should initialize with the correct _activityStreamPrerender value r=Mardak

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2ad95439eef6