Opened on 12/05/2014 at 08:40:19 AM
Closed on 01/07/2015 at 07:06:16 PM
Last modified on 05/20/2015 at 02:22:39 PM
#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/ |
||
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
- 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.
Attachments (0)
Change History (19)
comment:2 Changed on 12/05/2014 at 08:52:13 AM by trev
- Cc greiner added
comment:3 follow-up: ↓ 4 Changed on 12/05/2014 at 10:13:23 AM 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:6 Changed on 12/08/2014 at 12:27:53 PM by trev
- Owner set to trev
comment:7 Changed on 12/08/2014 at 12:29:54 PM by saroyanm
- Cc saroyanm added
comment:9 Changed on 12/16/2014 at 02:11:21 PM 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 on 12/16/2014 at 09:27:55 PM by trev
- Blocking 1706 added
comment:11 Changed on 12/16/2014 at 09:33:39 PM by trev
- Blocking 1708 added
comment:12 Changed on 12/17/2014 at 06:33:58 PM by trev
- Review URL(s) modified (diff)
comment:13 Changed on 12/17/2014 at 10:02:00 PM by trev
Pushed the contents of the first two reviews:
https://hg.adblockplus.org/adblockplusui/rev/e7ed2f3b7548
https://hg.adblockplus.org/adblockplusui/rev/51a82b693cde
https://hg.adblockplus.org/adblockplusui/rev/7f7000702aee
https://hg.adblockplus.org/adblockplusui/rev/2af64c1e429c
https://hg.adblockplus.org/adblockplusui/rev/af1b44e15433
comment:14 Changed on 12/19/2014 at 01:34:40 PM by trev
Pushed contents of the third review:
comment:15 Changed on 12/19/2014 at 07:12:05 PM 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 on 01/06/2015 at 10:35:42 PM 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 on 01/07/2015 at 07:06:08 PM 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 on 01/07/2015 at 07:06:16 PM by trev
- Resolution set to fixed
- Status changed from reviewing to closed
comment:19 Changed on 05/20/2015 at 02:22:39 PM by philll
- Platform changed from Firefox/Firefox Mobile to Firefox
Made Firefox and Firefox mobile available as seperate platforms.
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.