1438367 - Make about:home and about:newtab use the same nsIAboutModule flags

Status: RESOLVED FIXED

(Firefox :: New Tab Page, enhancement, P3)

Description

[Ed Lee :Mardak]

In bug 1433324 we attempted to share the same flags between about:home and about:newtab

+static const uint32_t ACTIVITY_STREAM_FLAGS =
+  nsIAboutModule::ALLOW_SCRIPT |
+  nsIAboutModule::ENABLE_INDEXED_DB |
+  nsIAboutModule::URI_MUST_LOAD_IN_CHILD |
+  nsIAboutModule::URI_SAFE_FOR_UNTRUSTED_CONTENT;

Notably, MAKE_LINKABLE was removed from about:home as it wasn't on about:newtab.

Gijs noted that switching will require migrating the indexeddb store on one of the two possible pages as they're two different origins (moz-safe-about:home and about:newtab moving to potentially about:home or moz-safe-about:newtab).

> if we only get 1 option for linkable/unlinkable, then unlinkable is definitely the right choice, and picking linkable would break about:newtab's storage, picking unlinkable would break about:home's storage
> we could try to come up with a migration for the data we're breaking, I guess

I believe things will "work" without migration, but data could be lost, e.g., blocked snippets forgotten.

A related issue is that we're running into this because we have both about:home and about:newtab. Where going with just one of them could avoid some of the problems.

Comment 1

[:Gijs (he/him)]

If we fix the syncing then presumably we can get rid of things relatively painlessly by just copying the data from the about:newtab store...

Comment 2

[Ed Lee :Mardak]

k88hudson, with bug 1438368 storing snippets block list in main ActivityStreamStorage/indexeddb, is the one in content still being used? If so, if we remove MAKE_LINKABLE from about:home, would the data from main get synced to the about:home origin (and just ignore the data in moz-safe-about:home origin)?

Comment 3

[Daniel Veditz [:dveditz]]

This is a bug, not an enhancement--about:home should not be linkable.

Comment 4

[Kate Hudson :k88hudson]

The blocklist in indexedDB in content is just a mirror of of the blocklist that comes from ActivityStreamStorage, so changing the origin should be fine. indexedDB in content is still depended on for caching the response from the snippets server, but it gets replaced every four hours anyway.

Comment 5

[Ed Lee :Mardak]

Gijs, given comment 4, is it okay to remove MAKE_LINKABLE that we split off from bug 1433324? I don't quite recall if that was the only concern then…

I pushed to try the ACTIVITY_STREAM_FLAGS part that was removed from the original patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a8a60a66fcbb5071e6da786ce13e1defe3d50a9a

Comment 6

[:Gijs (he/him)]

(In reply to Ed Lee :Mardak from comment #5)
> Gijs, given comment 4, is it okay to remove MAKE_LINKABLE that we split off
> from bug 1433324? I don't quite recall if that was the only concern then…

I think so. It's possible some tests might complain; I don't know off-hand but I'm sure your trypush will clarify that soon enough. :-)

If we don't already, we should probably make sure we have a follow-up to merge the two URLs. There's not really a great reason to have two anymore. :-)

Comment 7

[Ed Lee :Mardak]

(In reply to :Gijs (he/him; out 3-8 May) from comment #6)
> but I'm sure your trypush will clarify that soon enough. :-)
I believe I already fixed up tests that expected about:home to be linkable in the previous bug, and try seems to be good.

> If we don't already, we should probably make sure we have a follow-up to merge the two URLs. There's not really a great reason to have two anymore. :-)
Filed bug 1458933

Comment 8

[Ed Lee :Mardak]

Attachment: Bug 1438367 - Make about:home and about:newtab use the same nsIAboutModule flags.

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

Comment 9

[Ed Lee :Mardak]

I grabbed a build from the trypush and confirmed that opening page info says it's about:blank instead of about:home.

Comment 10

[:Gijs (he/him)]

Comment on attachment 8972912 [details]
Bug 1438367 - Make about:home and about:newtab use the same nsIAboutModule flags.

https://reviewboard.mozilla.org/r/241460/#review247520

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2a5a941c2db9
Make about:home and about:newtab use the same nsIAboutModule flags. r=Gijs

Comment 12

[Cosmin Sabou [:CosminS]]

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

Comment 13

[Ed Lee :Mardak]

This got reverted on 62 and 61 when trying to fix bug 1459655 crash.

Comment 14

[Ed Lee :Mardak]

Fixed by bug 1461407
https://hg.mozilla.org/mozilla-central/rev/7cef5b202339

Comment 15

[Ryan VanderMeulen (PTO, back July 27)]

Re-landed for 61 in bug 1461407.