Bug 147537 – Web Inspector: Convert CSSStyleDeclarationSection to ES6 Class

• Status: RESOLVED
• Resolution: INVALID
• Component: Web Inspector

[Devin Rousso]

The rest of the CSSStyleSidebar classes are ES6 classes except for CSSStyleDeclarationSection.  We should change that.

[Devin Rousso]

Created attachment 258027 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 258027 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=258027&action=review

r=me

> Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:28
> +    constructor(delegate, style) {

Style: Treat "constructor" like a method, and put the opening brace on its own line.

> Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:33
> +        console.assert(style);

Nit: I know this is not new, but it is better to assert not just that "style" exists, but that it adheres to some expected type. Perhaps:

    console.assert(style instanceof WebInspector.CSSStyleDeclaration);

[Devin Rousso]

Created attachment 258157 [details]
Patch

[Joseph Pecoraro]

Note, if you have already gotten a review, you can put up the new patch with the "reviewed by" line filled in, and just cq+ it for the commit-queue to land.

[Devin Rousso]

(In reply to comment #4)
> Note, if you have already gotten a review, you can put up the new patch with
> the "reviewed by" line filled in, and just cq+ it for the commit-queue to
> land.

I did not know that.  Thanks for the tip!

[Joseph Pecoraro]

Comment on attachment 258157 [details]
Patch

r=me

[Nikita Vasilyev]

(In reply to comment #4)
> Note, if you have already gotten a review, you can put up the new patch with
> the "reviewed by" line filled in, and just cq+ it for the commit-queue to
> land.

The review dropdown must be left empty. If you set it to "r+", the commit queue will fail.

[Timothy Hatcher]

Comment on attachment 258157 [details]
Patch

It looks like Brian got this. It is now a class in TOT.