Bug 58121

Summary: [WK2] layoutTestController.shadowRoot should return undefined when its argument is not an element
Product: WebKit Reporter: Dominic Cooney <dominicc>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben, commit-queue, dglazkov, dominicc
Priority: P2 Keywords: LayoutTestFailure, MakingBotsRed
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch none

Dominic Cooney
Reported 2011-04-08 00:25:31 PDT
For consistency with Mac DRT, and Chromium DRT (and GTK DRT after bug 58119), layoutTestController.shadowRoot should return undefined, not null, when its argument is not an element.
Attachments
Patch (1.76 KB, patch)
2011-04-08 00:27 PDT, Dominic Cooney 		no flags
DetailsFormatted DiffDiff
Patch (7.11 KB, patch)
2011-04-08 11:49 PDT, Dominic Cooney 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Dominic Cooney
Comment 1 2011-04-08 00:27:28 PDT
Created attachment 88779 [details] Patch
WebKit Commit Bot
Comment 2 2011-04-08 02:05:02 PDT
Comment on attachment 88779 [details] Patch Clearing flags on attachment: 88779 Committed r83270: <http://trac.webkit.org/changeset/83270>
WebKit Commit Bot
Comment 3 2011-04-08 02:05:05 PDT
All reviewed patches have been landed. Closing bug.
Adam Roben (:aroben)
Comment 4 2011-04-08 09:36:59 PDT
fast/dom/HTMLKeygenElement/keygen.html is still failing on WebKit2.
Adam Roben (:aroben)
Comment 5 2011-04-08 09:38:53 PDT
WKBundleNodeHandleCopyElementShadowRoot is returning 0, and LayoutTestController::shadowRoot returns JSValueMakeUndefined in that case.
Adam Roben (:aroben)
Comment 6 2011-04-08 09:42:24 PDT
I mean it returns JSValueMakeNull. But apparently the test wants JSValueMakeUndefined.
Adam Roben (:aroben)
Comment 7 2011-04-08 09:44:21 PDT
It looks like LayoutTestController doesn't have enough information to do the right thing. It's supposed to return undefined if it is passed a non-element node, but it has no way of determining whether a particular node is an element or not.
Adam Roben (:aroben)
Comment 8 2011-04-08 09:46:29 PDT
It's not clear to me that having shadowRoot return different things for non-elements and elements without shadow roots is worthwhile. Why do our tests need that behavior? The tests themselves are quite capable of figuring out what's an element and what isn't.
Dominic Cooney
Comment 9 2011-04-08 10:42:17 PDT
I will work on a patch to make the ports consistent and always return null.
Dominic Cooney
Comment 10 2011-04-08 11:49:35 PDT
Created attachment 88851 [details] Patch
Dimitri Glazkov (Google)
Comment 11 2011-04-08 13:15:01 PDT
Comment on attachment 88851 [details] Patch Clearing flags on attachment: 88851 Committed r83333: <http://trac.webkit.org/changeset/83333>
Dimitri Glazkov (Google)
Comment 12 2011-04-08 13:15:04 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.