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)

defect
Not set
normal

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.
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
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/
Blocks: 1259859
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+
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.
(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.
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
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.
Depends on: 1268912
You need to log in before you can comment on or make changes to this bug.