1340651 - Add a test that will look for improper use of 'Firefox' in dtd/properties files

Status: NEW

(Firefox :: General, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

+++ This bug was initially created as a clone of Bug #1259859 +++

We should have a script that runs on commit/check-in that looks at DTD/properties file changes and rejects the commit if 'Firefox' is used.

Comment 1

[Francesco Lodolo [:flod]]

I remember at least one case where Firefox was wanted, so you might need exception support. Finding that bug is going to be complicated though.

Comment 2

[Axel Hecht [:Pike]]

We have a good deal of those in overrides, where we override general toolkit/network strings to be used by any kind of gecko-based thing with Firefox specific versions.

As the gecko code generally doesn't require chrome://branding to exist, they don't do anything that would allow us to tell those code paths what our branding is.

For anything that's not an override, this sounds like a good thing to do, though.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Yeah, we have the ability to mark exceptions, and we will cover overrides. I'm not sure if we will cover strings that are masked by overrides.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

The test that I'm uploading will fail until bug 1340618 is fixed.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1340651 - Update our misused characters test to check for instances of Firefox that should be using brandShortName.

Review commit: https://reviewboard.mozilla.org/r/113542/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/113542/

Comment 6

[Florian Quèze [:florian]]

That's a lot of exceptions! Are we sure none of these are bugs that need fixing?

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Florian Quèze [:florian] [:flo] from comment #6)
> That's a lot of exceptions! Are we sure none of these are bugs that need
> fixing?

Most of those should have follow-up bugs to remove the exceptions. Most of the sync exceptions should be using syncBrand.dtd and most of the devtools exceptions should be using brandShortName. Though there are some things that we might want to keep as Firefox, or at least introduce another branding name for, such as the "Firefox Developer Tools" makes sense whereas "Developer Edition Developer Tools" doesn't make much sense.

Comment 8

[Florian Quèze [:florian]]

Comment on attachment 8838745 [details]
Bug 1340651 - Update our misused characters test to check for instances of Firefox that should be using brandShortName.

https://reviewboard.mozilla.org/r/113542/#review115160

Please include comments with bug numbers above the exceptions that we would like to see removed soon (will be r+ with that fixed).