The default bug view has changed. See this FAQ.

Enable brace-style and no-multi-spaces eslint rules for toolkit

RESOLVED FIXED in Firefox 53

Status

()

Toolkit
General
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: jaws, Assigned: jaws)

Tracking

unspecified
mozilla53
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Comment hidden (empty)
Comment hidden (mozreview-request)
Comment on attachment 8822809 [details]
Bug 1326511 - Enable brace-style and no-multi-spaces eslint rules for toolkit.

https://reviewboard.mozilla.org/r/101594/#review102122

In the future it would be good to separate the --fix from the manual fixes and separate the two rule changes.

::: browser/base/content/browser.js:5703
(Diff revision 1)
> -                                                return ""; });
> + changed = true;
> +                                                return "";

Identation seems off here

::: browser/base/content/utilityOverlay.js:122
(Diff revision 1)
>   *    (Currently, the Alt isn't sent here at all for menu items, but that will change in bug 126189.)
>   * - Alt is hard to use in context menus, because pressing Alt closes the menu.
>   * - Alt can't be used on the bookmarks toolbar because Alt is used for "treat this as something draggable".
>   * - The button is ignored for the middle-click-paste-URL feature, since it's always a middle-click.
>   */
> -function whereToOpenLink( e, ignoreButton, ignoreAlt )
> +function whereToOpenLink( e, ignoreButton, ignoreAlt ) {

There's extra spacing around these args which should also be normalized at some point. Feel free to fix this here if you want while you're touching the line.

::: browser/base/content/utilityOverlay.js:570
(Diff revision 1)
>    }
>    return true;
>  }
>  
>  // Gather all descendent text under given document node.
> -function gatherTextUnder( root )
> +function gatherTextUnder( root ) {

I guess there's more of it in this fileā€¦

::: toolkit/components/satchel/test/test_bug_511615.html:96
(Diff revision 1)
> -  dnEvent.initKeyEvent("keydown",  true, true, null, false, false, false, false, alwaysval, 0);
> +  dnEvent.initKeyEvent("keydown", true, true, null, false, false, false, false, alwaysval, 0);
>    prEvent.initKeyEvent("keypress", true, true, null, false, false, false, false, keycode, charcode);
> -  upEvent.initKeyEvent("keyup",    true, true, null, false, false, false, false, alwaysval, 0);
> +  upEvent.initKeyEvent("keyup", true, true, null, false, false, false, false, alwaysval, 0);

FYI: extra spaces were used in test files to align arguments and you will be breaking that. One option would be to add more exceptions to the rule for test directories.
Attachment #8822809 - Flags: review?(MattN+bmo) → review+
Comment hidden (mozreview-request)

Comment 4

3 months ago
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/06698dfb3784
Enable brace-style and no-multi-spaces eslint rules for toolkit. r=MattN

Comment 5

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/06698dfb3784
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.