1709522 - WinToast license is not included in about:license

Status: RESOLVED FIXED

(Toolkit :: General, defect, P3)

Description

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

MOZ_DEFAULT_BROWSER_AGENT is not defined under some circumstances, so the check (also here) to include the WinToast license fails.

We build and package license.html twice: once in omni.ja (chrome/toolkit/content/global/license.html), which does include the WinToast license, once in browser/omni.ja (chrome/browser/content/browser/license.html), which doesn't (though it does include this override). The one in browser/omni.js is the one shown in about:license.

• Why are the defines different?
• Why are there two copies shipped with only two differences? (see also bug 751599)
• Why does about:license show the browser file even though it seems to be mapped to global?

Comment 1

[Dave Townsend [:mossop]]

Mike, can you suggest a severity for this?

Comment 2

[Mike Hoye [:mhoye]]

Let's call it a P3. We should fix it promptly, but it isn't on fire.

What's our motivation for using MOZ_DEFAULT_BROWSER_AGENT here rather than XP_WIN ?

And is the simplest solution here for us to replace MOZ_DEFAULT_BROWSER_AGENT - in the license file only - with XP_WIN ?

Comment 3

[:Gijs (he/him)]

(In reply to Adam Gashlin (he/him) [:agashlin] (ex-moco) from comment #0)

MOZ_DEFAULT_BROWSER_AGENT is not defined under some circumstances, so the check (also here) to include the WinToast license fails.

We build and package license.html twice: once in omni.ja (chrome/toolkit/content/global/license.html), which does include the WinToast license, once in browser/omni.ja (chrome/browser/content/browser/license.html), which doesn't (though it does include this override). The one in browser/omni.js is the one shown in about:license.

• Why are the defines different?

because the jar preprocessor is running twice and running from a different build directory, so the defines in that build directory apply.

• Why are there two copies shipped with only two differences? (see also bug 751599)

Because we're dumb. But I'll leave fixing that to bug 751599 - we don't want to not have a license file for toolkit/ use, and it's somewhat tricky to come up with a generic way of including arbitrary other licenses into the toolkit version, I guess?

• Why does about:license show the browser file even though it seems to be mapped to global?

Because it gets overridden here.

(In reply to Mike Hoye [:mhoye] from comment #2)

Let's call it a P3. We should fix it promptly, but it isn't on fire.

FWIW, a P3 in Fx::General or Toolkit::General is... unlikely to get fixed, for lack of person-power and because the set of bugs that meets that criteria is not meaningfully different from "infinite" and therefore almost impossible to prioritize. If this needs fixing "promptly" then we should probably give it a higher prio.

What's our motivation for using MOZ_DEFAULT_BROWSER_AGENT here rather than XP_WIN ?

We don't use or ship WinToast outside of the default browser agent.

And is the simplest solution here for us to replace MOZ_DEFAULT_BROWSER_AGENT - in the license file only - with XP_WIN ?

Well, there are at least the following options:

• make the define available in browser/base where the jar preprocessor for the browser license runs. This has the downside that it's code addition, but it's The Right Thing, sort of.
• switch to XP_WIN because it's less work, but it'd be wrong-ish in that we'd include the license even if we build without the default browser agent
• get rid of MOZ_DEFAULT_BROWSER_AGENT and make it impossible to build without it.

I'm not aware of any reason we would want to not ship the default browser agent, and it's not like there's linux distros or something who would compile with different build flags. So the last option might be easiest... but I'm also not involved with the default browser agent, so checking in with Nick in case I'm missing stuff.

Comment 4

[Nick Alexander :nalexander [he/him]]

(In reply to :Gijs (he/him) from comment #3)

(In reply to Adam Gashlin (he/him) [:agashlin] (ex-moco) from comment #0)

MOZ_DEFAULT_BROWSER_AGENT is not defined under some circumstances, so the check (also here) to include the WinToast license fails.

We build and package license.html twice: once in omni.ja (chrome/toolkit/content/global/license.html), which does include the WinToast license, once in browser/omni.ja (chrome/browser/content/browser/license.html), which doesn't (though it does include this override). The one in browser/omni.js is the one shown in about:license.

• Why are the defines different?

because the jar preprocessor is running twice and running from a different build directory, so the defines in that build directory apply.

• Why are there two copies shipped with only two differences? (see also bug 751599)

Because we're dumb. But I'll leave fixing that to bug 751599 - we don't want to not have a license file for toolkit/ use, and it's somewhat tricky to come up with a generic way of including arbitrary other licenses into the toolkit version, I guess?

• Why does about:license show the browser file even though it seems to be mapped to global?

Because it gets overridden here.

(In reply to Mike Hoye [:mhoye] from comment #2)

Let's call it a P3. We should fix it promptly, but it isn't on fire.

FWIW, a P3 in Fx::General or Toolkit::General is... unlikely to get fixed, for lack of person-power and because the set of bugs that meets that criteria is not meaningfully different from "infinite" and therefore almost impossible to prioritize. If this needs fixing "promptly" then we should probably give it a higher prio.

What's our motivation for using MOZ_DEFAULT_BROWSER_AGENT here rather than XP_WIN ?

We don't use or ship WinToast outside of the default browser agent.

And is the simplest solution here for us to replace MOZ_DEFAULT_BROWSER_AGENT - in the license file only - with XP_WIN ?

Well, there are at least the following options:

• make the define available in browser/base where the jar preprocessor for the browser license runs. This has the downside that it's code addition, but it's The Right Thing, sort of.
• switch to XP_WIN because it's less work, but it'd be wrong-ish in that we'd include the license even if we build without the default browser agent
• get rid of MOZ_DEFAULT_BROWSER_AGENT and make it impossible to build without it.

I'm not aware of any reason we would want to not ship the default browser agent, and it's not like there's linux distros or something who would compile with different build flags. So the last option might be easiest... but I'm also not involved with the default browser agent, so checking in with Nick in case I'm missing stuff.

I learned some things about license.html, so I can't say you've missed anything :) WDBA is specific to browser/, so I think we should make the define available in browser/base. This is our hair shirt, we might as well wear it proudly.

Comment 5

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1709522 - Include WinToast license in `browser/omni.ja`, not in `/omni.ja`. r?gijs

Make chrome/browser/content/browser/license.html include the
WinToast license if the WDBA is enabled and never include the WinToast
license in chrome/toolkit/content/global/license.html.

While here, fix the anchor, which is case sensitive.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f27f7a2c880d
Include WinToast license in `browser/omni.ja`, not in `/omni.ja`. r=Gijs

Comment 7

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/f27f7a2c880d