Bug 147658 - Web Inspector: Convert miscellaneous view widgets to use ES6 classes
Summary: Web Inspector: Convert miscellaneous view widgets to use ES6 classes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: BJ Burg
URL:
Keywords:
Depends on:
Blocks: 142891
  Show dependency treegraph
 
Reported: 2015-08-04 15:45 PDT by BJ Burg
Modified: 2015-08-05 11:24 PDT (History)
7 users (show)

See Also:


Attachments
Proposed Fix (96.11 KB, patch)
2015-08-04 17:43 PDT, BJ Burg
joepeck: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2015-08-04 15:45:10 PDT
Toolbar, ContentBrowser, and whatever else is left.
Comment 1 BJ Burg 2015-08-04 17:43:50 PDT
Created attachment 258245 [details]
Proposed Fix
Comment 2 Joseph Pecoraro 2015-08-04 22:20:24 PDT
Comment on attachment 258245 [details]
Proposed Fix

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

r=me

> Source/WebInspectorUI/UserInterface/Base/Test.js:112
> +    constructor()
> +    {
> +        super();
> +    }

Just drop this constructor so we can test "default constructor" behavior of JSC.

> Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationSection.js:-2
> - * Copyright (C) 2013 Apple Inc. All rights reserved.

I believe Devin just put up a patch for this class. Perhaps your patch can just supersede it.

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

Nit: instanceof.

> Source/WebInspectorUI/UserInterface/Views/ContentBrowser.js:85
> +    }
>      // Public

Style: Empty line.

> Source/WebInspectorUI/UserInterface/Views/QuickConsoleNavigationBar.js:31
> +    constructor(element, navigationItems)
> +    {
> +        super(element, navigationItems);
> +    }

Nit: I think this could be the default constructor. Would be a very good test for JavaScriptCore to just drop this constructor!
Comment 3 BJ Burg 2015-08-05 11:24:31 PDT
Committed r187968: <http://trac.webkit.org/changeset/187968>