Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#1663 closed defect (fixed)

First-run page is broken with e10s enabled

Reported by: trev Assignee: trev
Priority: P1 Milestone:
Module: User-Interface Keywords:
Cc: tschuster, greiner, saroyanm Blocked By:
Blocking: #1706, #1708 Platform: Firefox
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

http://codereview.adblockplus.org/4875452820750336/
http://codereview.adblockplus.org/6427347985104896/
http://codereview.adblockplus.org/6180766664884224/
http://codereview.adblockplus.org/4731979438227456/
http://codereview.adblockplus.org/6432313504169984/

Description (last modified by trev)

Environment

Firefox 37.0a1 nightly build with e10s enabled (browser.tabs.remote.autostart.1 preference set to true).

How to reproduce

  1. Open chrome://adblockplus/content/ui/firstRun.html in the browser.

Observed behaviour

Links don't work, all "features" are shown as being "on" and clicking the buttons has no effect. The issue is apparently that require() returns undefined - observer notifications aren't a good way to reach the core code any more, it is running in a different process now.

Expected behaviour

Links should work correctly, same as the feature toggles.

Implementation notes

We need to make the first-run page communicate asynchronously with the core code. This means implementing ext.backgroundPage.sendMessage() and ext.onMessage.addListener() for Firefox. At this occasion, it would make sense to move the first-run page (including translations) the [https://hg.adblockplus.org/adblockplusui/ new adblockplusui repository. The actual messages being exchanged there should be in line with the plans for #1524.

Change History (19)

comment:1 Changed 5 years ago by trev

  • Description modified (diff)

comment:2 Changed 5 years ago by trev

  • Cc greiner added

I guess that asynchronous communication with the core code would the best solution here, along the same lines as what @greiner is implementing for the preferences UI. It shouldn't be too much effort either here.

comment:3 follow-up: Changed 5 years ago by greiner

Since #1524 looks like it could take a while still, we could introduce the messaging interface for the Firefox first-run page first. Some of that code could then be reused for the Platform options page.
Would the idea then be to already move the first-run page into the upcoming adblockplusui repository?

comment:4 in reply to: ↑ 3 Changed 5 years ago by trev

Replying to greiner:

Would the idea then be to already move the first-run page into the upcoming adblockplusui repository?

I think that will make most sense.

Note that #1524 isn't about Firefox anyway, so the first-run page will definitely become the test balloon here.

comment:5 Changed 5 years ago by trev

  • Description modified (diff)

comment:6 Changed 5 years ago by trev

  • Owner set to trev

comment:7 Changed 5 years ago by saroyanm

  • Cc saroyanm added

comment:8 Changed 5 years ago by trev

  • Description modified (diff)

comment:9 Changed 5 years ago by trev

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

I've split up the changes here into three reviews, to make them easier to follow.

comment:10 Changed 5 years ago by trev

  • Blocking 1706 added

comment:11 Changed 5 years ago by trev

  • Blocking 1708 added

comment:12 Changed 5 years ago by trev

  • Review URL(s) modified (diff)

comment:14 Changed 5 years ago by trev

comment:15 Changed 5 years ago by trev

Last review finished as well:

https://hg.adblockplus.org/adblockplusui/rev/80095e6961d0

At this point the first-run page is good enough for Chrome, in Firefox there are still some problems to fix however so I am leaving this issue open.

comment:16 Changed 5 years ago by trev

  • Review URL(s) modified (diff)

Added one last review, that one should address all issues discovered while testing in Firefox.

comment:17 Changed 5 years ago by trev

  • Component changed from Adblock-Plus-for-Firefox to User-Interface

Pushed the last batch of changes: https://hg.adblockplus.org/adblockplusui/rev/3bfa3974bccd

The new first-run page should be integrated into Firefox and Chrome/Opera/Safari shortly in #1706 and #1708 respectively.

comment:18 Changed 5 years ago by trev

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:19 Changed 5 years ago by philll

  • Platform changed from Firefox/Firefox Mobile to Firefox

Made Firefox and Firefox mobile available as seperate platforms.

Note: See TracTickets for help on using tickets.