Closed Bug 1478168 Opened 3 years ago Closed 3 years ago

Port bug 1476333 - Replace use of pref browser.chromeURL

Categories

(Thunderbird :: General, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 63.0

People

(Reporter: jorgk-bmo, Assigned: jorgk-bmo)

Details

Attachments

(1 file)

... since it's gone.
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?
Flags: needinfo?(bgrinstead)
(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.
Flags: needinfo?(bgrinstead)
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.
Attachment #8994689 - Flags: feedback?(bgrinstead) → feedback+
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
Attachment #8994689 - Flags: feedback?(geoff)
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/3d1872a693cf
Port 1476333: Replace use of pref browser.chromeURL. rs=bustage-fix, f=bgrins DONTBUILD
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 63.0
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.
Attachment #8994689 - Flags: review+
(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.
Summary: Port 1476333 - Replace use of pref browser.chromeURL → Port bug 1476333 - Replace use of pref browser.chromeURL
You need to log in before you can comment on or make changes to this bug.