1478168 - Port bug 1476333 - Replace use of pref browser.chromeURL

Status: RESOLVED FIXED

(Thunderbird :: General, enhancement)

Description

[Jorg K (CEST = GMT+2)]

... since it's gone.

Comment 1

[Jorg K (CEST = GMT+2)]

No, I didn't look properly, M-C removed it, but we still have it:
https://dxr.mozilla.org/comm-central/search?q=browser.chromeURL&redirect=false.

Also see:
https://dxr.mozilla.org/comm-central/search?q=getBrowserURL&redirect=false

Brian, now that bug 1476333 has landed, do we need to port anything?

Comment 2

[Brian Grinstead [:bgrins]]

(In reply to Jorg K (GMT+2) from comment #1)
> No, I didn't look properly, M-C removed it, but we still have it:
> https://dxr.mozilla.org/comm-central/search?q=browser.
> chromeURL&redirect=false.
> 
> Also see:
> https://dxr.mozilla.org/comm-central/search?q=getBrowserURL&redirect=false
> 
> Brian, now that bug 1476333 has landed, do we need to port anything?

Yes, you'll need to define BROWSER_CHROME_URL in confvars.sh as in https://hg.mozilla.org/mozilla-central/rev/f4181dab3ad8#l1.13 and then change accesses of the pref to use `AppConstants.BROWSER_CHROME_URL`.

Looks like getBrowserURL() directly reads the pref so as long as that implementation changes to return the AppConstants value you probably don't have to change callers of that function.

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: 1476333-browser.chromeURL.patch (v1)

Maybe something like this. Taken from:
https://hg.mozilla.org/mozilla-central/rev/dd386b5b9fa7#l38.12
https://hg.mozilla.org/mozilla-central/rev/dd386b5b9fa7#l2.12
https://hg.mozilla.org/mozilla-central/rev/f4181dab3ad8#l1.13

Comment 4

[Brian Grinstead [:bgrins]]

Comment on attachment 8994689 [details] [diff] [review]
1476333-browser.chromeURL.patch (v1)

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

::: mail/app/profile/all-thunderbird.js
@@ -13,5 @@
>  
>  pref("general.skins.selectedSkin", "classic/1.0");
>  
>  #ifdef XP_MACOSX
> -pref("browser.chromeURL", "chrome://messenger/content/messengercompose/messengercompose.xul");

Huh, why is this only defined in OSX? I guess `getBrowserURL()` is never called in non-OSX? Regardless, it seems OK to define it unconditionally in the build.

Comment 5

[Jorg K (CEST = GMT+2)]

Thanks, Brian. My latest push after the landing of bug 1476333 had no errors, so as you said, getBrowserURL() is likely not used in TB. I'll land this anyway to keep in sync with FF. I assume it won't break anything, but better safe than sorry:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=b0bd2bb12762317fc39dbfe2392a3aba05379342

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/3d1872a693cf
Port 1476333: Replace use of pref browser.chromeURL. rs=bustage-fix, f=bgrins DONTBUILD

Comment 7

[:aceman]

Comment on attachment 8994689 [details] [diff] [review]
1476333-browser.chromeURL.patch (v1)

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

Thanks.
Do we actually need the getBrowserURL function now? May m-c code call our version of it?
If not, we could probably switch the 2 users of it in c-c to the new constant and remove the function.

Comment 8

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #7)
> Do we actually need the getBrowserURL function now?
That's a good question. I had the impression it might be something like you added in bug 1427188:
https://hg.mozilla.org/comm-central/rev/dd64bc33f829#l3.14

To investigate this, we'd need to add some dump() and then check the logs in automation.