1517496 - Capstone: Make misused string test (browser_misused_characters_in_strings.js) also check fluent files

Status: RESOLVED FIXED

(Firefox :: General, enhancement, P3)

Description

[:Gijs (he/him)]

We have an automated mochitest-browser-chrome test that checks whether people use straight quotes (', ") in (en-US versions of) localized strings, where they should prefer curly quotes. It also checks for "..." instead of the unicode ellipsis character. It has a list of exceptions at the top of the file.

https://searchfox.org/mozilla-central/source/browser/base/content/test/static/browser_misused_characters_in_strings.js

To make this work, we'll need to iterate through the zip file for the ftl files, like we do for .properties files:

https://searchfox.org/mozilla-central/source/browser/base/content/test/static/browser_misused_characters_in_strings.js#221
  let uris = await getAllTheFiles(".properties");

and then parse each file by doing something like:

Cu.import("resource://gre/modules/Fluent.jsm");
for (let uri of uris) {
  let rawContents = await fetchFile(uri);
  let resource = FluentResource.fromString(rawContents);
  for (let val of resource.values()) {
    // lint `val` here.
  }
}

Note that a value could be a single string, or a nested array with terms or other objects. We should check all the strings, but can ignore terms and other objects the parser returns.

Comment 1

[Francesco Lodolo [:flod]]

We already have a bug on file (bug 1416149). I don't mind duping it one way or the other.

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

> Note that a value could be a single string, or a nested array with terms or other objects. We should check all the strings, but can ignore terms and other objects the parser returns.

That's an interesting use case. You may prefer to use `fluent-syntax` for that [0]. Fluent Syntax is a full parser/AST/serializer for FTL resources which produces a clean AST tree specifically for tooling purposes such as this linter, rather than a runtime optimized entries.

This may be important because we do not make any guarantees about the entries model and treat it as an internal implementation detail. AST, on the other hand, is more public and stable at this point.

In result, if you use the runtime parser (from Fluent.jsm) and try to introspect the entries structure it may change from under you in the future. We can try to remember to keep it in sync, but it's fishy.

The issue with fluent-syntax is that it's not yet vendored in. I'm not sure if the pros outweigh the cons here, your call :)

What I do know is that as we move closer to use fluent-rs instead of fluent.js, we'll move to use the AST as in `fluent-syntax` for runtime as well, so writing it to walk the AST seems like a more forward-compatible way.


[0] https://github.com/projectfluent/fluent.js/tree/master/fluent-syntax

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

Stas, Pike - thoughts?

Comment 4

[:Gijs (he/him)]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #2)
> > Note that a value could be a single string, or a nested array with terms or other objects. We should check all the strings, but can ignore terms and other objects the parser returns.
> 
> That's an interesting use case. You may prefer to use `fluent-syntax` for
> that [0]. Fluent Syntax is a full parser/AST/serializer for FTL resources
> which produces a clean AST tree specifically for tooling purposes such as
> this linter, rather than a runtime optimized entries.
> 
> This may be important because we do not make any guarantees about the
> entries model and treat it as an internal implementation detail. AST, on the
> other hand, is more public and stable at this point.
> 
> In result, if you use the runtime parser (from Fluent.jsm) and try to
> introspect the entries structure it may change from under you in the future.
> We can try to remember to keep it in sync, but it's fishy.

Well, the test will just fail if we make incompatible changes. I think that's fine.

> The issue with fluent-syntax is that it's not yet vendored in. I'm not sure
> if the pros outweigh the cons here, your call :)

I don't think they do, as importing an entire node module for this seems like overkill. TBH, considering terms aren't going to have quotes or ellipses in themselves, I half considered suggesting we just read the entire value and call it a day. Anyway, using FluentSource seems easy enough and is cleaner.

We can always update the test code if we end up removing Fluent.jsm / FluentSource and its not-quite-ast. But in practice, I'm hopeful we fix bug 1416149 before that happens.

Comment 5

[Axel Hecht [:Pike]]

I'm not sure if we should actually cargo-cult the existing test forward. The mix is hand-woven parsers there never thrilled me. Also the fact that we're going through built files.

The exception logic is also leaving room for improvement, for example by using dedicated substrings to exclude. That way, we could have a test that enforces pretty quotes outside of the element in strings like `The “coords” attribute of the <area shape="circle"> tag is not in the “center-x,center-y,radius” format.`, https://dxr.mozilla.org/mozilla-central/rev/c2593a3058afdfeaac5c990e18794ee8257afe99/dom/locales/en-US/chrome/layout/layout_errors.properties#6.

Maybe it's a good idea to start building this out as a linter based on compare-locales and project config, with an API to get .... string fragments? Integrating a python code fragment into our linter mach setup is gonna be easier than js, too, https://firefox-source-docs.mozilla.org/tools/lint/create.html

In contrast, I think that "just support fluent in what we have" is not a lot less work than doing the linter right from the start.

Comment 6

[Staś Małolepszy :stas]

Please don't use Fluent.jsm's FluentResource.fromString for parsing. As Zibi said, the data structure which holds translations may change without warning in a future update. The changes might be subtle enough to not trigger test failures.

fluent-syntax has been written specifically for the parsing use-case. It produces the same AST for all syntax constructs regardless of where in the message they are. This isn't the case with FluentResource.fromString which takes shortcuts and optimizes the data structures for speed and memory savings.

The AST produced by fluent-syntax is defined in https://github.com/projectfluent/fluent.js/blob/master/fluent-syntax/src/ast.js and is also easily previewable at https://projectfluent.org/play/. Fluent.jsm doesn't have either of these facilities.

fluent-syntax is around 50KB of code with no dependencies (it's a single file). If you don't want to vendor it, perhaps this test could be written in Python? third_party/python/fluent is already in the tree and is functionally the same.

I see Axel's point about implementing this as a linter. This is tempting but I don't agree with the assessment that it would be only slightly more work than adapting the current test. Adapting it is not going to yield perfect results but could be useful in the short term. I would defer to Flod and his judgement on how useful this test has been in the past.

Comment 7

[Francesco Lodolo [:flod]]

(In reply to Staś Małolepszy :stas from comment #6)

I would defer to Flod and his judgement on how useful this test has been in the past.

It is useful. We moved away from me having to comment in each string about using consistent punctuation, to a test that fails even if I don't manage to see the patch before it lands. For reference, we already have at least one error in ftl files, even if I review each of the patches.

Comment 8

[:Gijs (he/him)]

(In reply to Staś Małolepszy :stas from comment #6)

Please don't use Fluent.jsm's FluentResource.fromString for parsing. As Zibi said, the data structure which holds translations may change without warning in a future update. The changes might be subtle enough to not trigger test failures.

This doesn't really make sense to me as an argument. This is shipped code. If it's insufficiently tested that's a problem completely irrespective of what we do here. I don't think the possibility that the format of the fromString output changes and doesn't cause this test to fail and then magically stops catching (some?) mistakes in strings is really all that likely. If we want, we can make the test be picky in order to ensure it gets updated if the fake-AST format changes.

fluent-syntax has been written specifically for the parsing use-case. It produces the same AST for all syntax constructs regardless of where in the message they are. This isn't the case with FluentResource.fromString which takes shortcuts and optimizes the data structures for speed and memory savings.

That actually makes FluentResource sound more, not less appealing. This test is a perennial performance drain, we don't even run bits of these static tests on debug/asan because they're just too slow.

The AST produced by fluent-syntax is defined in https://github.com/projectfluent/fluent.js/blob/master/fluent-syntax/src/ast.js and is also easily previewable at https://projectfluent.org/play/. Fluent.jsm doesn't have either of these facilities.

Sure, but it's not like the AST isn't going to change either, because Fluent is still changing... Plus then we get to play "vendor in the new version of fluent-syntax and deal with any test fallout" game every so often. I already get to do this for Readability / reader mode, it's not fun. A 50k new library to do that with is not appealing.

fluent-syntax is around 50KB of code with no dependencies (it's a single file).

https://github.com/projectfluent/fluent.js/tree/master/fluent-syntax/src doesn't look like a single file to me...

Also, what is https://searchfox.org/mozilla-central/source/intl/l10n/fluent.js.patch ?

Also keep in mind that browser tests do not have any NPM or require() infrastructure.

If you don't want to vendor it, perhaps this test could be written in Python? third_party/python/fluent is already in the tree and is functionally the same.

We don't have any python unit tests. Only marionette, which isn't a unit-test framework but an integration test one, which isn't really the right choice here. Plus then we have to re-invent the wheel in terms of the iteration over files (inside jars...), devtools filtering, exception lists, and everything else.

On the whole, I'm not really convinced that "just use fluent-syntax" is as painless as is being suggested.

I agree that long-term, a linter is a better suggestion, but I agree with Staś that it's not as easy, and I really think we want a stopgap until then (esp. given comment #7). Fixing this bug using FluentResource should be a relatively trivial way to do that, for the students to get familiar with fluent and our infrastructure, and we can migrate to a linter after that. Then the whole thing goes away anyway.

Comment 9

[:Gijs (he/him)]

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

let resource = FluentResource.fromString(rawContents);
for (let val of resource.values()) {
// lint val here.
}
}

Would it be better if we used resource.keys() here and then just called await Localization.formatMessages() with it, setting it up with the same bundle? Or expose a "more public" API on Localization.jsm that gets key/string pairs given a bundle URI?

Comment 10

[Zibi Braniecki [:zbraniecki][:gandalf]]

Would it be better if we used resource.keys() here and then just called await Localization.formatMessages() with it, setting it up with the same bundle? Or expose a "more public" API on Localization.jsm that gets key/string pairs given a bundle URI?

This would only give you one variant of the message in cases where the value has multiple variants. I assume the test is meant to inspect all variants.

Comment 11

[Staś Małolepszy :stas]

Thanks for your comments, Gijs. I see where you're coming from and I'll try to address your concerns inline. My recommendation is still to choose fluent-syntax over Fluent.jsm for this test.

This doesn't really make sense to me as an argument. This is shipped code. If it's insufficiently tested that's a problem completely irrespective of what we do here.

It's well tested but for a different purpose: formatting messages to strings. Nowhere in the tests do we actually verify the shape of the data structure. What we do is we check if the formatted string looks right.

It's code which shipped, but it's also private API; any changes in the future will not be mentioned in the changelog and may be done in a patch release. Which also creates an additional risk when uplifting updates to Fluent.jsm. If there's a critical bug in FluentBundle which we fix by changing the (internal and private) data structure for storing the translations, we'd also need to fix this test before it can land.

I don't think the possibility that the format of the fromString output changes and doesn't cause this test to fail and then magically stops catching (some?) mistakes in strings is really all that likely.

You're probably right, thanks for pointing it out. The test will serve as a litmus paper for AST changes anyways. The benefit of using fluent-syntax is that those changes are also planned and documented in the changelog which makes it possible to 1) plan an update to the test, and 2) easily understand what that update should look like.

That actually makes FluentResource sound more, not less appealing. This test is a perennial performance drain, we don't even run bits of these static tests on debug/asan because they're just too slow.

fluent-syntax's FluentParser is slower than FluentResource (because it's more precise and produces good error information), but it's still fast. We're likely talking a few milliseconds per file. Parsing all 50 Fluent files in gecko-strings on my machine takes 65 milliseconds in total with FluentParser (vs. 7 ms with FluentResource). With FluentParser, developer time is saved too because traversing the AST tree is easy when its structure is well-defined and consistent. And the risk of it breaking in the future because of an internal change in FluentResource is 0.

The AST produced by fluent-syntax is defined in https://github.com/projectfluent/fluent.js/blob/master/fluent-syntax/src/ast.js and is also easily previewable at https://projectfluent.org/play/. Fluent.jsm doesn't have either of these facilities.

Sure, but it's not like the AST isn't going to change either, because Fluent is still changing...

I'm happy to say that this is much less of a concern now. As of December, Fluent has entered the stabilization phase (see https://discourse.mozilla.org/t/fluent-syntax-0-8/34604). The AST might still change slightly before Fluent 1.0 is published, but we don't plan any more changes to the syntax at this point. After 1.0, we're going to commit to the syntax and the AST for all the 1.x releases of Fluent. We are not going to commit to the runtime AST produced by FluentResource, however.

Plus then we get to play "vendor in the new version of fluent-syntax and deal with any test fallout" game every so often. I already get to do this for Readability / reader mode, it's not fun.

That's also true in the FluentResource scenario, isn't it? And as I mentioned above, in that scenario we're risking increased test fallout for each update of Fluent.jsm. With fluent-syntax, we'd decouple updates of Fluent.jsm (which is code on the critical user-facing path) from the AST updates which affect this test.

fluent-syntax is around 50KB of code with no dependencies (it's a single file).

https://github.com/projectfluent/fluent.js/tree/master/fluent-syntax/src doesn't look like a single file to me...

We would vendor in the bundled version of the code, just as we do with Fluent.jsm, Localization.jsm etc.

Also, what is https://searchfox.org/mozilla-central/source/intl/l10n/fluent.js.patch ?

It keeps track of changes which are applied manually to the product of fluent.js build system to make it work in Gecko.

Also keep in mind that browser tests do not have any NPM or require() infrastructure.

Right, we'd need to at least add EXPORTED_SYMBOLS to the bundled FluentSyntax.jsm.

If you don't want to vendor it, perhaps this test could be written in Python? third_party/python/fluent is already in the tree and is functionally the same.

We don't have any python unit tests. Only marionette, which isn't a unit-test framework but an integration test one, which isn't really the right choice here. Plus then we have to re-invent the wheel in terms of the iteration over files (inside jars...), devtools filtering, exception lists, and everything else.

OK, thanks for providing more detail here. Looks like going with the JS version is the way to go and the question is whether to use FluentResource or fluent-syntax's full AST FluentParser.

Comment 12

[:Gijs (he/him)]

(In reply to Staś Małolepszy :stas from comment #11)

It's code which shipped, but it's also private API;

It's exported from the JSM, doesn't have underscores or "private" or "internal" or anything like that in its name - how is anyone supposed to know they shouldn't use it?

any changes in the future will not be mentioned in the changelog and may be done in a patch release. Which also creates an additional risk when uplifting updates to Fluent.jsm. If there's a critical bug in FluentBundle which we fix by changing the (internal and private) data structure for storing the translations, we'd also need to fix this test before it can land.

If it were really critical, you could always disable that part of the test, certainly on beta/release/esr which is string-frozen anyway.

That actually makes FluentResource sound more, not less appealing. This test is a perennial performance drain, we don't even run bits of these static tests on debug/asan because they're just too slow.

fluent-syntax's FluentParser is slower than FluentResource (because it's more precise and produces good error information), but it's still fast. We're likely talking a few milliseconds per file. Parsing all 50 Fluent files in gecko-strings on my machine takes 65 milliseconds in total with FluentParser (vs. 7 ms with FluentResource). With FluentParser, developer time is saved too because traversing the AST tree is easy when its structure is well-defined and consistent. And the risk of it breaking in the future because of an internal change in FluentResource is 0.

This doesn't really address the overhead from going over the more complex AST as opposed to the simplified, faster thing FluentResource provides, but even without that "use the thing that's 10 times slower" doesn't make me enthusiastic...

Plus then we get to play "vendor in the new version of fluent-syntax and deal with any test fallout" game every so often. I already get to do this for Readability / reader mode, it's not fun.

That's also true in the FluentResource scenario, isn't it?

I didn't think it was - there's no indication in the file that it's mirrored from somewhere else or that the canonical repository is not mozilla-central. I assumed Fluent.jsm was mozilla-central-only code. Looking around now, it seems there's a readme which clarifies this, but this isn't obvious when just doing code searches. Who is responsible for merging updates? Can we add disclaimers to the relevant files? Note that hg log for the other JSMs indicates both "update to version X" commits and other fixes that were committed through "regular" m-c changes. What process is used to ensure we don't clobber m-c only changes when updating?

I think I could live with using FluentSyntax if the l10n team took responsibility for vendoring in a .jsm version of it that is packaged testing-only (ie not shipped as part of Firefox) and ensured it got updated as part of any other updates of the other fluent jsms - though it's still unfortunate that we'd end up with 2 versions of "is this valid fluent"...

Does that sound acceptible?

Comment 13

[Axel Hecht [:Pike]]

I don't see us accepting ownership on this test, tbh. At least I strongly feel against that.

FTR, I don't think we'll do anything short term here, the code we're writing will just stick, just as the original version of this test stuck for years.

If we had an implementation based on source files as a linter, we could also use that outside of Firefox. And I really don't see the point in enumerating the jar a couple of times.

Comment 14

[:Gijs (he/him)]

(In reply to Axel Hecht [:Pike] from comment #13)

I don't see us accepting ownership on this test, tbh. At least I strongly feel against that.

That's not what I asked. I asked if you could accept ownership of the vendoring of FluentSyntax. Those are not the same thing, right?

Comment 15

[Staś Małolepszy :stas]

The l10n team already owns Fluent.jsm and friends, as well as third_party/python/fluent, and we make sure to update them all when new releases are published. I'm up for doing the same for fluent-syntax (or, more specifically, FluentSyntax.jsm, or whatever we call it in the tree).

Comment 16

[Yuan Cheng]

Attachment: Bug 1517496 - Make misused string test also check fluent files, r?Gijs,jaws

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/12662799de6c
Make misused string test also check fluent files, r=jaws

Comment 18

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset 12662799de6c (Bug 1517496) for ES lint failure and :jaws request

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=12662799de6c6dee77e6f443adfbfc45f51fe597&selectedJob=226595007

Backout link: https://hg.mozilla.org/integration/autoland/rev/84272e744f16292325d580723a9ba17de7b6b8f2

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=226595007&repo=autoland&lineNumber=309

[vcs 2019-02-06T20:17:59.953Z] TinderboxPrint:<a href=https://hg.mozilla.org/integration/autoland/rev/12662799de6c6dee77e6f443adfbfc45f51fe597 title='Built from autoland revision 12662799de6c6dee77e6f443adfbfc45f51fe597'>12662799de6c6dee77e6f443adfbfc45f51fe597</a>
[task 2019-02-06T20:17:59.953Z] executing ['bash', '-cx', 'cd /builds/worker/checkouts/gecko/ && cp -r /build/node_modules_eslint node_modules && ln -s ../tools/lint/eslint/eslint-plugin-mozilla node_modules && ln -s ../tools/lint/eslint/eslint-plugin-spidermonkey-js node_modules && ./mach lint -l eslint -f treeherder --quiet\n']
[task 2019-02-06T20:17:59.957Z] + cd /builds/worker/checkouts/gecko/
[task 2019-02-06T20:17:59.957Z] + cp -r /build/node_modules_eslint node_modules
[task 2019-02-06T20:18:00.244Z] + ln -s ../tools/lint/eslint/eslint-plugin-mozilla node_modules
[task 2019-02-06T20:18:00.246Z] + ln -s ../tools/lint/eslint/eslint-plugin-spidermonkey-js node_modules
[task 2019-02-06T20:18:00.246Z] + ./mach lint -l eslint -f treeherder --quiet
[task 2019-02-06T20:18:01.077Z] New python executable in /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python2.7
[task 2019-02-06T20:18:01.077Z] Also creating executable in /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python
[task 2019-02-06T20:18:02.734Z] Installing setuptools, pip, wheel...done.
[task 2019-02-06T20:18:03.804Z] running build_ext
[task 2019-02-06T20:18:03.804Z] building 'psutil._psutil_linux' extension
[task 2019-02-06T20:18:03.804Z] creating build
[task 2019-02-06T20:18:03.804Z] creating build/temp.linux-x86_64-2.7
[task 2019-02-06T20:18:03.804Z] creating build/temp.linux-x86_64-2.7/psutil
[task 2019-02-06T20:18:03.804Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2019-02-06T20:18:03.804Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2019-02-06T20:18:03.804Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_linux.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o
[task 2019-02-06T20:18:03.804Z] creating build/lib.linux-x86_64-2.7
[task 2019-02-06T20:18:03.804Z] creating build/lib.linux-x86_64-2.7/psutil
[task 2019-02-06T20:18:03.804Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so
[task 2019-02-06T20:18:03.804Z] building 'psutil._psutil_posix' extension
[task 2019-02-06T20:18:03.804Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2019-02-06T20:18:03.804Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2019-02-06T20:18:03.804Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so
[task 2019-02-06T20:18:03.804Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so -> psutil
[task 2019-02-06T20:18:03.804Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so -> psutil
[task 2019-02-06T20:18:03.804Z]
[task 2019-02-06T20:18:03.804Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2019-02-06T20:23:35.231Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/base/content/test/static/browser_misused_characters_in_strings.js:266:26 | Please use ChromeUtils.import instead of Cu.import (mozilla/use-chromeutils-import)
[taskcluster 2019-02-06 20:23:35.577Z] === Task Finished ===

Comment 19

[Zibi Braniecki [:zbraniecki][:gandalf]]

The way this test is written now will only work for Messages which have no attributes and no placeables.

Any message that has any expression, or attributes, will not work at all:

key = The username is { $username }.

key2 = { $num ->
  [one] This is a value for one
 *[other] This is a value for other
}

key3 = This is a value
  .tooltip = Foo

I'm wondering if this test should land without any support for all of those cases.

Comment 20

[:Gijs (he/him)]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #19)

The way this test is written now will only work for Messages which have no attributes and no placeables.

This isn't true of the patch as it is in phabricator today.

I'm wondering if this test should land without any support for all of those cases.

Yes, it should. Something is better than nothing, and we can iterate on it. Ditto for FluentSyntax support, see the discussion in phabricator.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b1514d57bcdc
Make misused string test also check fluent files, r=jaws,flod

Comment 22

[Razvan Maries]

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