Bug 161792 – TextTrackLoader should use FetchOptions::mode according its crossOrigin attribute

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[youenn fablet]

TextTrackLoader should use FetchOptions::mode according its crossOrigin attribute

[youenn fablet]

Created attachment 288397 [details]
Patch

[youenn fablet]

(In reply to comment #1)
> Created attachment 288397 [details]
> Patch

Patch is changing the behavior in the case of no crossOrigin attribute and URL is cross-origin. Before the patch, access was refused.
With the patch, access is ok.
This seems to align with https://html.spec.whatwg.org/multipage/embedded-content.html#sourcing-out-of-band-text-tracks

Also, this check is not all that useful as it could be bypassed by a same-origin URL that would redirect to a cross-origin one.

I hesitated to keep or not that check.
Eric, do you know the rationale behind it?

[Build Bot]

Comment on attachment 288397 [details]
Patch

Attachment 288397 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2041158

New failing tests:
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrack/addCue.html
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/id.html
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/endTime.html
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/pauseOnExit.html
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/startTime.html
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/track.html
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrack/removeCue.html

[Build Bot]

Created attachment 288398 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[youenn fablet]

Created attachment 288404 [details]
Rebasing failing tests

[youenn fablet]

(In reply to comment #3)
> Comment on attachment 288397 [details]
> Patch
> 
> Attachment 288397 [details] did not pass mac-ews (mac):
> Output: http://webkit-queues.webkit.org/results/2041158
> 
> New failing tests:
> imported/w3c/web-platform-tests/html/semantics/embedded-content/media-
> elements/interfaces/TextTrack/addCue.html
> imported/w3c/web-platform-tests/html/semantics/embedded-content/media-
> elements/interfaces/TextTrackCue/id.html
> imported/w3c/web-platform-tests/html/semantics/embedded-content/media-
> elements/interfaces/TextTrackCue/endTime.html
> imported/w3c/web-platform-tests/html/semantics/embedded-content/media-
> elements/interfaces/TextTrackCue/pauseOnExit.html
> imported/w3c/web-platform-tests/html/semantics/embedded-content/media-
> elements/interfaces/TextTrackCue/startTime.html
> imported/w3c/web-platform-tests/html/semantics/embedded-content/media-
> elements/interfaces/TextTrackCue/track.html
> imported/w3c/web-platform-tests/html/semantics/embedded-content/media-
> elements/interfaces/TextTrack/removeCue.html

They were failing as the src was data URLs.
This allows passing some more tests.

The change of behavior is due to the removed check.
In terms of interoperability, this allows aligning with Firefox and the spec.
But this is breaking with Chromium and past WebKit behavior.

[Alex Christensen]

Comment on attachment 288404 [details]
Rebasing failing tests

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

> Source/WebCore/ChangeLog:15
> +        Also, this check could be bypassed in the case of a same-origin URL redirecting to a cross-origin one.

You mean it could before this patch but not after this patch, right?

[youenn fablet]

(In reply to comment #7)
> Comment on attachment 288404 [details]
> Rebasing failing tests
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=288404&action=review
> 
> > Source/WebCore/ChangeLog:15
> > +        Also, this check could be bypassed in the case of a same-origin URL redirecting to a cross-origin one.
> 
> You mean it could before this patch but not after this patch, right?

Yes, if we really wanted to enforce this check for redirections, we should set fetch mode to same-origin.

[Eric Carlson]

(In reply to comment #2)
> (In reply to comment #1)
> > Created attachment 288397 [details]
> > Patch
> 
> Patch is changing the behavior in the case of no crossOrigin attribute and
> URL is cross-origin. Before the patch, access was refused.
> With the patch, access is ok.
> This seems to align with
> https://html.spec.whatwg.org/multipage/embedded-content.html#sourcing-out-of-
> band-text-tracks
> 
> Also, this check is not all that useful as it could be bypassed by a
> same-origin URL that would redirect to a cross-origin one.
> 
> I hesitated to keep or not that check.
> Eric, do you know the rationale behind it?

The code was added for the original spec text, see bug 73184.

[WebKit Commit Bot]

Comment on attachment 288404 [details]
Rebasing failing tests

Clearing flags on attachment: 288404

Committed r205750: <http://trac.webkit.org/changeset/205750>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.