Closed
Bug 1268159
Opened 8 years ago
Closed 8 years ago
Use GreD in addition to XCurProcD for browser_misused_characters_in_strings.js
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox49 | --- | affected |
People
(Reporter: jaws, Assigned: jaws)
References
Details
Attachments
(1 file)
GreD covers more string files and is showing 338 failures in this test when GreD is used instead in the checkAllProperties function.
Assignee | ||
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/49309/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/49309/
Attachment #8746216 -
Flags: review?(gijskruitbosch+bugs)
Assignee | ||
Comment 2•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3cbb753c754
Assignee | ||
Updated•8 years ago
|
Summary: Use GreD instead of XCurProcD for browser_misused_characters_in_strings.js → Use GreD in addition to XCurProcD for browser_misused_characters_in_strings.js
Assignee | ||
Comment 3•8 years ago
|
||
Comment on attachment 8746216 [details] MozReview Request: Bug 1268159 - Use GreD in addition to XCurProcD for browser_misused_characters_in_strings.js to cover more string files. r?gijs Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49309/diff/1-2/
Comment 4•8 years ago
|
||
Comment on attachment 8746216 [details] MozReview Request: Bug 1268159 - Use GreD in addition to XCurProcD for browser_misused_characters_in_strings.js to cover more string files. r?gijs https://reviewboard.mozilla.org/r/49309/#review46261 As try has noted, you also need to update https://dxr.mozilla.org/mozilla-central/source/browser/base/content/test/general/browser_aboutCertError.js#228 which depends on the error message whose quotes you're modifying. ::: browser/base/content/test/general/browser_misused_characters_in_strings.js:180 (Diff revision 2) > + let urisGreD = yield generateURIsFromDirTree(appDirGreD, [extension]); > + let urisXCurProcD = yield generateURIsFromDirTree(appDirXCurProcD, [extension]); I think you can avoid work here in some cases by using nsIFile's `contains` method to check if one of the two dirs contains the other, rather than doing some of the same work twice and then using a `Set` to remove the dupes. Certainly here on OS X, on a beta distributed build, `GreD` is simply the parent directory of `XCurProcD`. ::: browser/base/content/test/general/browser_misused_characters_in_strings.js:182 (Diff revision 2) > + let uris = Array.from(new Set(urisGreD.concat(appDirXCurProcD))); > + return uris; Nit: no need for the 'uris' temp, AFAICT. ::: toolkit/locales/en-US/chrome/mozapps/handling/handling.properties:10 (Diff revision 2) > protocol.title=Launch Application > protocol.description=This link needs to be opened with an application. > protocol.choices.label=Send to: > protocol.checkbox.label=Remember my choice for %S links. > protocol.checkbox.accesskey=R > -protocol.checkbox.extra=This can be changed in %S's preferences. > +protocol.checkbox.extra=This can be changed in %S’s preferences. Nit: kill trailing whitespace please.
Attachment #8746216 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment 5•8 years ago
|
||
Can you clarify what's the rationale between ‘%1$S’ and “%1$S”? I have the feeling you automated replacing ' with ’, but in several cases “” should be used. Example: AutomaticAuth=You are about to log in to the site “%1$S” with the username “%2$S”. PEVariableEmpty=Expected variable value but found ‘%1$S’. To be clear, I'm perfectly fine with it, just wanted to understand if there's a reason or it's just a script's expected outcome.
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #5) > Can you clarify what's the rationale between ‘%1$S’ and “%1$S”? I have the > feeling you automated replacing ' with ’, but in several cases “” should be > used. Example: > > AutomaticAuth=You are about to log in to the site “%1$S” with the username > “%2$S”. > PEVariableEmpty=Expected variable value but found ‘%1$S’. > > To be clear, I'm perfectly fine with it, just wanted to understand if > there's a reason or it's just a script's expected outcome. We haven't formulated the recommended quote usage yet. I maintained the current quote usage, replacing single straight quotes with single curly quotes and double straight quotes with double curly quotes.
Assignee | ||
Comment 7•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/0af3c129a3665692271cda977178286d5937e326 Bug 1268159 - Use GreD in addition to XCurProcD for browser_misused_characters_in_strings.js to cover more string files. r=gijs
Assignee | ||
Comment 8•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/cc898e5dfa22a5263e34488baccd32c2ca275f30 Bug 1268159 - followup, update some tests that got missed in the conversion from straight quotes to curly quotes and revert the Sync client name change until a follow-up bug can track down the deeper regression. r=me on a CLOSED TREE.
Assignee | ||
Comment 9•8 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/0af3c129a366 http://hg.mozilla.org/mozilla-central/rev/cc898e5dfa22
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•