Bug 160097 - First parameter to Window.getComputedStyle() should be mandatory and non-nullable
Summary: First parameter to Window.getComputedStyle() should be mandatory and non-null...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: WebExposed
Depends on: 161206
Blocks:
  Show dependency treegraph
 
Reported: 2016-07-22 13:34 PDT by Chris Dumez
Modified: 2016-08-25 13:07 PDT (History)
8 users (show)

See Also:


Attachments
Patch (16.72 KB, patch)
2016-07-22 13:48 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2016-07-22 13:34:10 PDT
First parameter to Window.getComputedStyle() should be mandatory and non-nullable:
- https://drafts.csswg.org/cssom/#extensions-to-the-window-interface

Firefox and Chrome agree with the specification.
Comment 1 Chris Dumez 2016-07-22 13:48:16 PDT
Created attachment 284370 [details]
Patch
Comment 2 WebKit Commit Bot 2016-07-22 15:57:15 PDT
Comment on attachment 284370 [details]
Patch

Clearing flags on attachment: 284370

Committed r203623: <http://trac.webkit.org/changeset/203623>
Comment 3 WebKit Commit Bot 2016-07-22 15:57:22 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Darin Adler 2016-07-22 22:03:32 PDT
Comment on attachment 284370 [details]
Patch

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

> Source/WebCore/css/CSSComputedStyleDeclaration.h:51
> +    ComputedStyleExtractor(RefPtr<Node>&&, bool allowVisitedStyle = false, PseudoId = NOPSEUDO);

Does this have to work on a Node, or maybe just on an Element? Does this have to take a RefPtr, or maybe just a Ref?

> Source/WebCore/inspector/InspectorCSSAgent.cpp:646
> +    RefPtr<CSSComputedStyleDeclaration> computedStyleInfo = CSSComputedStyleDeclaration::create(*element, true);

Should use auto.

> Source/WebCore/page/DOMWindow.h:231
> +        RefPtr<CSSStyleDeclaration> getComputedStyle(Element&, const String& pseudoElt) const;

Should return Ref, not RefPtr.

> Source/WebCore/testing/Internals.h:110
> +    RefPtr<CSSComputedStyleDeclaration> computedStyleIncludingVisitedInfo(Element&) const;

Should return Ref, not RefPtr.
Comment 5 Simon Fraser (smfr) 2016-07-23 12:23:39 PDT
This broke the windows build, as EWS indicates:

C:\cygwin\home\buildbot\slave\win-debug\build\Source\WebKit\win\DOMCoreClasses.cpp(804): error C2664: 'WTF::RefPtr<WebCore::CSSStyleDeclaration> WebCore::DOMWindow::getComputedStyle(WebCore::Element &,const WTF::String &) const': cannot convert argument 1 from 'WebCore::Element *' to 'WebCore::Element &' [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\WebKit\WebKit.vcxproj]
C:\cygwin\home\buildbot\slave\win-debug\build\Source\WebKit\win\DOMCoreClasses.cpp(804): error C2228: left of '.get' must have class/struct/union [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\WebKit\WebKit.vcxproj]
Comment 6 Chris Dumez 2016-07-23 14:08:47 PDT
(In reply to comment #5)
> This broke the windows build, as EWS indicates:
> 
> C:\cygwin\home\buildbot\slave\win-
> debug\build\Source\WebKit\win\DOMCoreClasses.cpp(804): error C2664:
> 'WTF::RefPtr<WebCore::CSSStyleDeclaration>
> WebCore::DOMWindow::getComputedStyle(WebCore::Element &,const WTF::String &)
> const': cannot convert argument 1 from 'WebCore::Element *' to
> 'WebCore::Element &'
> [C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\Source\WebKit\WebKit.vcxproj]
> C:\cygwin\home\buildbot\slave\win-
> debug\build\Source\WebKit\win\DOMCoreClasses.cpp(804): error C2228: left of
> '.get' must have class/struct/union
> [C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\Source\WebKit\WebKit.vcxproj]

I'll fix shortly. Thanks.
Comment 7 Chris Dumez 2016-07-23 14:11:43 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > This broke the windows build, as EWS indicates:
> > 
> > C:\cygwin\home\buildbot\slave\win-
> > debug\build\Source\WebKit\win\DOMCoreClasses.cpp(804): error C2664:
> > 'WTF::RefPtr<WebCore::CSSStyleDeclaration>
> > WebCore::DOMWindow::getComputedStyle(WebCore::Element &,const WTF::String &)
> > const': cannot convert argument 1 from 'WebCore::Element *' to
> > 'WebCore::Element &'
> > [C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\Source\WebKit\WebKit.vcxproj]
> > C:\cygwin\home\buildbot\slave\win-
> > debug\build\Source\WebKit\win\DOMCoreClasses.cpp(804): error C2228: left of
> > '.get' must have class/struct/union
> > [C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\Source\WebKit\WebKit.vcxproj]
> 
> I'll fix shortly. Thanks.

<http://trac.webkit.org/changeset/203651> should fix the Windows build.