Closed Bug 1414018 Opened 2 years ago Closed 2 years ago

Redirect current tab to new New Tab after disabling New Tab extension in doorhanger

Categories

(WebExtensions :: Frontend, defect, P3)

defect

Tracking

(firefox59 verified)

VERIFIED FIXED
mozilla59
Tracking Status
firefox59 --- verified

People

(Reporter: mstriemer, Assigned: mstriemer)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

When the "Restore Settings" button is clicked in the New Tab doorhanger the current tab is (normally) closed. Instead we should replace the current tab with what the New Tab would be after the change.

Disabling an extension is supposed to close all of the tabs associated with that extension, which is why the current tab closes. To handle this we may need to:

1. Redirect the tab to about:blank so it doesn't close.
2. Disable the extension (the tab should stay open since it's at about:blank).
3. Once the New Tab Service has been updated, redirect to new tab.
4. Ensure the doorhanger appears for the next extension, if needed.

If we know what URL we need to redirect to beforehand we can likely just do steps 3 and 4 above.
Depends on: 1390158
Priority: -- → P3
Blocks: 1342584
Here's how this would look if you install two New Tab extensions then open a new tab and restore both.
Comment on attachment 8934314 [details]
Bug 1414018 - Go to new New Tab page after disabling add-on in New Tab doorhanger

https://reviewboard.mozilla.org/r/205218/#review211594

::: browser/components/extensions/test/browser/browser_ext_url_overrides_newtab.js:42
(Diff revision 1)
>      }
>      Services.obs.addObserver(observer, eventName);
>    });
>  }
>  
> +function waitForURL(url) {

can you just use `BrowserTestUtils.browserLoaded()` or `BrowserTestUtils.waitForLocationChange()` here?

::: browser/components/extensions/test/browser/browser_ext_url_overrides_newtab.js:449
(Diff revision 1)
> +  // FIXME: We need to enable the add-on so it gets cleared from the
> +  // ExtensionSettingsStore for now. See bug 1408226.

didn't this get fixed with bug 1404584?
Attachment #8934314 - Flags: review?(aswan) → review+
Comment on attachment 8934314 [details]
Bug 1414018 - Go to new New Tab page after disabling add-on in New Tab doorhanger

https://reviewboard.mozilla.org/r/205218/#review211594

> didn't this get fixed with bug 1404584?

This is still an issue because we're using `callOnClose()` to update `ExtensionSettingsStore`. That just maps to a shutdown listener which will only get called once. In that case if the extension is disabled then removed the store is still in the disabled state.
Keywords: checkin-needed
Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8d680183148c
Go to new New Tab page after disabling add-on in New Tab doorhanger r=aswan
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8d680183148c
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Attached image tabs restore.gif
I can reproduce this issue on Firefox 58.0a1 (20171102222620) under Wind 10 64-bit. 

This issue is verified as fixed on Firefox 59.0a1 (20171227100314) under Wind 10 64-bit and Mac OS X 10.13.
Status: RESOLVED → VERIFIED
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.