1482342 - Themes installation tab should open links in Thunderbird

Status: RESOLVED FIXED

(Thunderbird :: General, enhancement)

Description

[Andrei Hajdukewycz [:sancus]]

Right now the link "Choose from thousands of themes" on the "Themes" tab in the Addons Manager opens in Thunderbird, but many subsequent links such as theme detail pages and other clicks open in the browser, making it confusing to install these.

This tab should open all ATN links in Thunderbird like the discovery pane tab does.

Comment 1

[Andrei Hajdukewycz [:sancus]]

I added a couple of CCs because I don't really know how to fix this. The link in question, "Choose from thousands of themes" on the Themes tab, is created by setting an href here: https://searchfox.org/comm-central/source/mozilla/toolkit/mozapps/extensions/content/extensions.js#2340 on the label in extensions.xul: https://searchfox.org/comm-central/source/mozilla/toolkit/mozapps/extensions/content/extensions.xul#289

Now the thing is, as far as I understand, since the Addons Manager is opened by openContentTab here: https://searchfox.org/comm-central/source/mail/base/content/mailCore.js#480 all of the clicks should end up going through the specialTabs siteClickHandler defined here: https://searchfox.org/comm-central/source/mail/base/content/utilityOverlay.js#272

But when you click this link, it starts opening the new tab before even entering that function, and aEvent.defaultPrevented is true so it never even checks anything inside of siteClickHandler here: https://searchfox.org/comm-central/source/mail/base/content/specialTabs.js#1126

So I'm not really sure where to go from here. I don't know how to override the behaviour for this tab when I can't even figure out what is handling its' opening.

Comment 2

[Magnus Melin [:mkmelin]]

I notice for contentTab_onTabOpened in specialTabs.js
https://searchfox.org/comm-central/rev/cce392b6a57170eb2139bbf15eaa9a88340f97b2/mail/base/content/specialTabs.js#726 when clicking the link to see all the themes

 openTab aArgs={"contentPage":"https://addons.thunderbird.net/en-US/thunderbird/themes/?src=thunderbird","clickHandler":null} 

Dunno why that is, but if there's no clickHandler there, no click handler is going to run

Comment 3

[Andrei Hajdukewycz [:sancus]]

OK, so it seems like the siteClickHandler only actually applies to the initial Discovery Pane tab... it doesn't apply to any of the other tabs. Not sure why that is but will continue to investigate.

Comment 4

[Andrei Hajdukewycz [:sancus]]

Attachment: Keep clicks on Themes browsing page in Thunderbird

Comment 5

[Andrei Hajdukewycz [:sancus]]

Comment on attachment 9006153 [details] [diff] [review]
Keep clicks on Themes browsing page in Thunderbird

OK well this patch seems really hacky, but after discussing it with several people I can't figure out a better way of doing this.

The default click handler for these links in: https://searchfox.org/mozilla-central/source/toolkit/content/widgets/text.xml#292 overrides the original click handler we set when opening the Add-ons Manager and there doesn't seem to be any reasonable way of stopping that.

In fact the click handling for the Add-ons Manager probably needs to be rewritten so every single click in it is handled by our own custom function, because most of the Firefox logic is bad for us because we don't want new tabs opening in and out of Thunderbird at random, but I have no idea how to approach that, and it probably is too much for this bug.

Feedback more than welcome!

Comment 6

[Geoff Lankow (:darktrojan)]

Comment on attachment 9006153 [details] [diff] [review]
Keep clicks on Themes browsing page in Thunderbird

Review of attachment 9006153 [details] [diff] [review]:
-----------------------------------------------------------------

I've no better idea.

::: mail/base/content/specialTabs.js
@@ +762,5 @@
> +      // loads some new URLs, so we add it back in here for the Themes browsing page.
> +      let themesBrowseURL = Services.urlFormatter.formatURLPref("extensions.getAddons.themes.browseURL");
> +      let addonRegExp = Services.prefs.getStringPref("extensions.getAddons.siteRegExp");
> +      if (aArgs.contentPage === themesBrowseURL) {
> +        aArgs.clickHandler = "specialTabs.siteClickHandler(event, new RegExp(\"" + addonRegExp + "\"));";

Maybe you should use ` for the string here so you can avoid escaping the ".

Comment 7

[Andrei Hajdukewycz [:sancus]]

Attachment: Keep clicks on Themes browsing page in Thunderbird

Comment 8

[Andrei Hajdukewycz [:sancus]]

Comment on attachment 9006183 [details] [diff] [review]
Keep clicks on Themes browsing page in Thunderbird

I changed to getCharPref because that's what I used in previous patches with this pref, and I don't want to create inconsistencies.

Other than that, same patch that darktrojan just r+'d.

Thanks for the help!

Comment 9

[Jorg K (CEST = GMT+2)]

Comment on attachment 9006183 [details] [diff] [review]
Keep clicks on Themes browsing page in Thunderbird

Looks good to me, but I'll land with Geoff's review.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/4b1de1461255
Open links from Themes browsing page in Thunderbird instead of external browser. r=darktrojan DONTBUILD

Comment 11

[Jorg K (CEST = GMT+2)]

TB 60.1 ESR:
https://hg.mozilla.org/releases/comm-esr60/rev/6543d3d17da247546027fd69f593cf526f51be0f