Bug 188226 – Optionally expose Attr::style to JavaScript

• Component: Bindings
• Status: RESOLVED FIXED

Attr::style is currently exposed via the Objective-C DOM bindings, but it’s not a standard property, so it’s not part of the JavaScript DOM API exposed to webpages.

To allow clients of the Objective-C DOM bindings to transition to the JavaScript bindings, add a runtime-enabled option to expose Attr::style to JavaScript as well.

<rdar://problem/42818113>

Created attachment 346298 [details]
Expose Attr::style to JS when _WKProcessPoolConfiguration.attrStyleEnabled is YES

[Darin Adler]

Comment on attachment 346298 [details]
Expose Attr::style to JS when _WKProcessPoolConfiguration.attrStyleEnabled is YES

This comment in Attr.cpp is now inaccurate:

CSSStyleDeclaration* Attr::style()
{
    // This function only exists to support the Obj-C bindings.

I suggest either removing the comment or replacing the comment with something that helps explain what context we might want to make this public (possibly handy for JavaScript code implementing editing operations for apps like Mail) and what context we should not (non-standard, not intended to be part of the web platform).

[EWS Watchlist]

Comment on attachment 346298 [details]
Expose Attr::style to JS when _WKProcessPoolConfiguration.attrStyleEnabled is YES

Attachment 346298 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8732735

New failing tests:
http/tests/security/canvas-remote-read-remote-video-blocked-no-crossorigin.html
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-video.html
http/tests/security/video-poster-cross-origin-crash2.html

[EWS Watchlist]

Created attachment 346356 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

Thanks for the review, Darin!

(In reply to Darin Adler from comment #3)
> Comment on attachment 346298 [details]
> Expose Attr::style to JS when _WKProcessPoolConfiguration.attrStyleEnabled
> is YES
> 
> This comment in Attr.cpp is now inaccurate:
> 
> CSSStyleDeclaration* Attr::style()
> {
>     // This function only exists to support the Obj-C bindings.
> 
> I suggest either removing the comment or replacing the comment with
> something that helps explain what context we might want to make this public
> (possibly handy for JavaScript code implementing editing operations for apps
> like Mail) and what context we should not (non-standard, not intended to be
> part of the web platform).

I replaced the comment with one that is correct, but probably doesn’t quite satisfy your requirements.

Committed <https://trac.webkit.org/r234502>.

[Dawei Fenton (:realdawei)]

(In reply to mitz from comment #6)
> Thanks for the review, Darin!
> 
> (In reply to Darin Adler from comment #3)
> > Comment on attachment 346298 [details]
> > Expose Attr::style to JS when _WKProcessPoolConfiguration.attrStyleEnabled
> > is YES
> > 
> > This comment in Attr.cpp is now inaccurate:
> > 
> > CSSStyleDeclaration* Attr::style()
> > {
> >     // This function only exists to support the Obj-C bindings.
> > 
> > I suggest either removing the comment or replacing the comment with
> > something that helps explain what context we might want to make this public
> > (possibly handy for JavaScript code implementing editing operations for apps
> > like Mail) and what context we should not (non-standard, not intended to be
> > part of the web platform).
> 
> I replaced the comment with one that is correct, but probably doesn’t quite
> satisfy your requirements.
> 
> Committed <https://trac.webkit.org/r234502>.

After this revision the macOS and iOS bots are showing TestWebKitAPI.WebKit.AttrStyle Timeout

Sample output:
https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20%28Tests%29/builds/5959/steps/run-api-tests/logs/stdio

(In reply to David Fenton (:realdawei) from comment #7)
> (In reply to mitz from comment #6)
> > 
> > Committed <https://trac.webkit.org/r234502>.
> 
> After this revision the macOS and iOS bots are showing
> TestWebKitAPI.WebKit.AttrStyle Timeout
> 
> Sample output:
> https://build.webkit.org/builders/
> Apple%20High%20Sierra%20Release%20WK2%20%28Tests%29/builds/5959/steps/run-
> api-tests/logs/stdio

Strange that we didn’t see it locally nor in EWS. I am guessing that the timeout occurs while waiting to load the test page.

(In reply to mitz from comment #8)
> (In reply to David Fenton (:realdawei) from comment #7)
> > (In reply to mitz from comment #6)
> > > 
> > > Committed <https://trac.webkit.org/r234502>.
> > 
> > After this revision the macOS and iOS bots are showing
> > TestWebKitAPI.WebKit.AttrStyle Timeout
> > 
> > Sample output:
> > https://build.webkit.org/builders/
> > Apple%20High%20Sierra%20Release%20WK2%20%28Tests%29/builds/5959/steps/run-
> > api-tests/logs/stdio
> 
> Strange that we didn’t see it locally nor in EWS. I am guessing that the
> timeout occurs while waiting to load the test page.

I downloaded the archive the bot was testing from <https://s3-us-west-2.amazonaws.com/archives.webkit.org/mac-highsierra-x86_64-release/234502.zip> and tried it myself:

$ DYLD_FRAMEWORK_PATH=~/Desktop/234502/Release ~/Desktop/234502/Release/TestWebKitAPI 
**PASS** WebKit.FormSubmission
**PASS** WebKit.AboutBlankLoad
**PASS** WebKit.AdditionalReadAccessAllowedURLs
**PASS** WebKit.AlwaysRevalidatedURLSchemes
**PASS** WebKit.ApplicationManifestCoding
**PASS** WebKit.ApplicationManifestBasic
**PASS** WebKit.ApplicationManifestDisplayMode
**PASS** WebKit.RespondToPolicyForNavigationResponseAsynchronously
**PASS** WebKit.PolicyForNavigationResponseCancelAsynchronously
**PASS** WebKit.AttrStyle
**PASS** WebKit.AutoLayoutIntegration
[…]

so the the test is passing on my computer with the same bits. I may need access to the bot to understand what’s different there.

Wenson figured it out!

Fixed in <https://trac.webkit.org/r234505>.

[Dawei Fenton (:realdawei)]

(In reply to mitz from comment #10)
> Wenson figured it out!
> 
> Fixed in <https://trac.webkit.org/r234505>.

Great!