Issue 5587 - Use options module

Issue 5587 - Use options module

[Manish Jethani, 2017-10-04 21:15:34]

Patch Set 1

Since we're moving ext.showOptions into a module, we need to update
adblockplusui here.

https://codereview.adblockplus.org/29564735/diff/29564736/ext/background.js
File ext/background.js (left):

https://codereview.adblockplus.org/29564735/diff/29564736/ext/background.js#o...
ext/background.js:83: if
(!/\/(?:mobile-)?options\.html/.test(top.location.href))
In the new version I've removed the "mobile-" part since we're planning to use
options.html as the top page always.

[Thomas Greiner, 2017-10-05 11:58:16]

https://codereview.adblockplus.org/29564735/diff/29564736/ext/background.js
File ext/background.js (left):

https://codereview.adblockplus.org/29564735/diff/29564736/ext/background.js#o...
ext/background.js:83: if
(!/\/(?:mobile-)?options\.html/.test(top.location.href))
On 2017/10/04 21:15:33, Manish Jethani wrote:
> In the new version I've removed the "mobile-" part since we're planning to use
> options.html as the top page always.

We're only using options.html as the top page inside the extension. For the test
environment this top page doesn't exist though which is why this check is
necessary.

[Manish Jethani, 2017-10-05 12:09:25]

Patch Set 2: Include check for mobile-options.html

https://codereview.adblockplus.org/29564735/diff/29564736/ext/background.js
File ext/background.js (left):

https://codereview.adblockplus.org/29564735/diff/29564736/ext/background.js#o...
ext/background.js:83: if
(!/\/(?:mobile-)?options\.html/.test(top.location.href))
On 2017/10/05 11:58:16, Thomas Greiner wrote:
> On 2017/10/04 21:15:33, Manish Jethani wrote:
> > In the new version I've removed the "mobile-" part since we're planning to
use
> > options.html as the top page always.
> 
> We're only using options.html as the top page inside the extension. For the
test
> environment this top page doesn't exist though which is why this check is
> necessary.

Ah, I see.

Made the change.

[Thomas Greiner, 2017-10-05 12:25:54]

LGTM

https://codereview.adblockplus.org/29564735/diff/29564736/ext/background.js
File ext/background.js (left):

https://codereview.adblockplus.org/29564735/diff/29564736/ext/background.js#o...
ext/background.js:83: if
(!/\/(?:mobile-)?options\.html/.test(top.location.href))
On 2017/10/05 12:09:25, Manish Jethani wrote:
> On 2017/10/05 11:58:16, Thomas Greiner wrote:
> > On 2017/10/04 21:15:33, Manish Jethani wrote:
> > > In the new version I've removed the "mobile-" part since we're planning to
> use
> > > options.html as the top page always.
> > 
> > We're only using options.html as the top page inside the extension. For the
> test
> > environment this top page doesn't exist though which is why this check is
> > necessary.
> 
> Ah, I see.
> 
> Made the change.

Thanks.