Skip to content
This repository has been archived by the owner on Feb 29, 2020. It is now read-only.

resource://activity-stream/data/content/activity-stream.bundle.js changes restored new tab title to "undefined" when switching to another tab #2819

Closed
MikkCZ opened this issue Jul 4, 2017 · 11 comments

Comments

@MikkCZ
Copy link

MikkCZ commented Jul 4, 2017

Also see https://bugzilla.mozilla.org/show_bug.cgi?id=1382139
I was unable to reproduce this with mozregression nor clean profile, but this still happens even after I have disabled all add-ons, so it's likely to happen for users with used profile when Activity Stream is in Firefox.

STR (see this video):

  1. Open Firefox. Set preference what to do when Firefox starts to open last tabs and windows.
  2. Open about:newtab and have it focused.
  3. Close Firefox.
  4. Start Firefox.
  5. Switch to another tab.
  6. Tab title changes to "undefined".

The cause is a call from Base.componentDidMount() on visibilitychange event to updateTitle() with _ref3 variable in uninitialized state. Personally I am not sure why this happens or in which cases should be the tab title changed at all. But the fix should be easy, check the _ref3 state or the string existence before it is actually used for document.title.

@Mardak
Copy link
Member

Mardak commented Jul 6, 2017

A similar thing should be running Firefox with "about:newtab" as a command line argument to have it open about:newtab as early as possible.

I can reproduce this on Linux (not OS X).

@Mardak
Copy link
Member

Mardak commented Jul 6, 2017

Actually, maybe I can't quite reproduce the same issue. I think I'm running into a separate issue where the page's window.sendAsyncMessage never exists? Are there any errors in the console?

@MikkCZ
Copy link
Author

MikkCZ commented Jul 6, 2017

I am running Linux Mint and my current build is Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0 ID:20170705170343 CSet: 11755fd63168581e194258d04bb6a7337779ec78

First I was looking in the console tab in DevTools, but actually there are these errors in the browser console right after Nightly starts up:

TypeError: this.sendAsyncMessage is not a function activity-stream.bundle.js:444:5

DataError: Data provided to an operation does not meet requirements. (neznámý)
wrapMethods/cls.prototype[method] resource://gre/modules/IndexedDB.jsm:103:47
get resource://gre/modules/ExtensionParent.jsm:1358:22
InterpretGeneratorResume self-hosted:1272:8
next self-hosted:1179:9

(It's a localized build, so (neznámý) means (unknown) here.)

@Mardak
Copy link
Member

Mardak commented Jul 6, 2017

Okay, so I might be running into the same thing locally then.

Just to confirm, the new tab page is almost empty except for the fox icon in the top left and the banner at the bottom of the page?

And just to double check, when you get into the state of "undefined" as the title, if you check window.sendAsyncMessage in the page Console, it is also undefined?

@MikkCZ
Copy link
Author

MikkCZ commented Jul 6, 2017

Well, maybe it's a slightly different behavior what we see after all. window.sendAsyncMessage is defined function both before and after I open Firefox and switch to another tab. But your first question pointed me a bit further. The new tab page is the activity stream page what is enabled in Nightly. I have hidden the search and top pages from it, but still there should be the gear icon in the top right corner. But when I open Firefox, it's not there in the restored tab, as can be seen on the video.

So I was playing around a bit and the problem only occurs when I have about:newtab also set as Firefox homepage in the preferences. If I have about:home (default), everything works as expected and even the TypeError mentioned above does not appear in the browser console after start, instead locale is undefined appears. So the actual STR for me are as follows (but still can reproduce only in my used profile for some reason):

  1. Open Firefox. Set preference what to do when Firefox starts to open last tabs and windows, and change homepage to about:newtab.
  2. Open about:newtab and have it focused.
  3. Close Firefox.
  4. Start Firefox (here you may notice about:home to flash on the screen for a very short moment).
  5. Switch to another tab.
  6. Tab title changes to "undefined".

When I change the Firefox homepage to something different from about:newtab, I cannot reproduce anymore. That's interesting, because Firefox should not care about the homepage preference and just load tabs from the last session. It seems to me, that Firefox displays the homepage according to the preferences first and than replaces it by the actual tab content. And for some reason, it fails, when those two are the same (about:newtab), or because of some race condition as about:newtab is definitely restored way way faster than a regular website.

@AdamHillier
Copy link
Contributor

We can stop the new tab title changing to "undefined" on tab-away by only adding the visibilitychange event listener (in componentDidMount() in Base.jsx) if the page is not already visible (i.e. document.visibilityState !== "visible").

This unfortunately does not solve the issue of the page content not rendering.

@AdamHillier
Copy link
Contributor

AdamHillier commented Jul 25, 2017

I think this might be caused by the channel (instance of RemotePages) in ActivityStreamMessageChannel being instantiated after an about:newtab page already exists resulting in messages not being sent to that page. As a result it only ever sees the initial empty state. Inspecting the 'blank' page shows that React is working correctly but as App.initialised is false no elements are rendered.

In my testing, when Firefox starts without the issue, then channel.messagePorts.size is always ${number of about:newtab pages} + 1 (the preloaded page). When it starts with the issue the number is always one fewer.

@AdamHillier
Copy link
Contributor

@Mardak
Copy link
Member

Mardak commented Jul 31, 2017

Sounds like we can avoid this issue by starting up earlier so that RemotePages can initialize before any about:newtab tabs are loaded. One way to do that is reverting 86547df but would then reintroduce talos sessionstore regressions. Also would be useful to note that moving startup earlier will probably break the test making sure various services/modules aren't loaded until later.

More complex would be to somehow register RemotePages early on while deferring the rest of ActivityStream.jsm and related loads. @k88hudson any ideas on how this can be done quickly and maybe not-so-complexly?

@Mardak
Copy link
Member

Mardak commented Jul 31, 2017

Oh, and Mossop just suggested having AboutNewTab just pass RemotePages instance off to ActivityStream... that's kinda neat.

@Mardak
Copy link
Member

Mardak commented Aug 2, 2017

@Mardak Mardak closed this as completed Aug 2, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.