Bug 123926 – [MSE] Validate the BCP47-ness of the language string passed to VideoTrack::setLanguage()

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

[MSE] Validate the BCP47-ness of the language string passed to VideoTrack::setLanguage()

[John Wilander]

Created attachment 297227 [details]
Patch

[Jer Noble]

Comment on attachment 297227 [details]
Patch

Nice! r=me

[Build Bot]

Comment on attachment 297227 [details]
Patch

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

New failing tests:
media/track/w3c/interfaces/TextTrack/language.html
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrack/language.html

[Build Bot]

Created attachment 297240 [details]
Archive of layout-test-results from ews102 for mac-yosemite

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

[Build Bot]

Comment on attachment 297227 [details]
Patch

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

New failing tests:
media/track/w3c/interfaces/TextTrack/language.html
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrack/language.html

[Build Bot]

Created attachment 297243 [details]
Archive of layout-test-results from ews117 for mac-yosemite

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

[Build Bot]

Comment on attachment 297227 [details]
Patch

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

New failing tests:
media/track/w3c/interfaces/TextTrack/language.html
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrack/language.html

[Build Bot]

Created attachment 297246 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

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

[Build Bot]

Comment on attachment 297227 [details]
Patch

Attachment 297227 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2731793

New failing tests:
media/only-bcp47-language-tags-accepted.html

[Build Bot]

Created attachment 297255 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[John Wilander]

I believe the W3C platform test is wrong (html/semantics/embedded-content/media-elements/interfaces/TextTrack/language.html).

Pull request:
https://github.com/w3c/web-platform-tests/pull/4359

[Chris Dumez]

Comment on attachment 297227 [details]
Patch

Based on discussion on the spec, I do not think we should land as is. The W3C test seems correct and should not fail.

[John Wilander]

After a discussion with Jer and Chris I will change this patch to only validate for internal purposes. This will allow the existing Web Platform test to continue working.

[John Wilander]

Created attachment 297503 [details]
Patch

[John Wilander]

Need a new review, please.

[Build Bot]

Comment on attachment 297503 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/html/dom/reflection-embedded.html
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/HTMLElement/HTMLTrackElement/srclang.html

[Build Bot]

Created attachment 297505 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

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

[Build Bot]

Comment on attachment 297503 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/html/dom/reflection-embedded.html
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/HTMLElement/HTMLTrackElement/srclang.html

[Build Bot]

Created attachment 297506 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 297503 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/HTMLElement/HTMLTrackElement/srclang.html

[Build Bot]

Created attachment 297508 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 297503 [details]
Patch

Attachment 297503 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2757291

New failing tests:
imported/w3c/web-platform-tests/html/dom/reflection-embedded.html
media/only-bcp47-language-tags-accepted-as-valid.html

[Build Bot]

Created attachment 297509 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Daniel Bates]

Comment on attachment 297503 [details]
Patch

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

> Source/WebCore/html/HTMLMediaElement.cpp:3777
> +        LOG(Media, "HTMLMediaElement::configureTextTrackGroup(%p) -  '%s' track with language '%s' and BCP 47 language '%s' has score %i", this, textTrack->kindKeyword().string().utf8().data(), textTrack->language().string().utf8().data(), textTrack->validBCP47Language().string().utf8().data(), trackScore);

Nit: Although the format specifier %i is equivalent to %d for formatting a signed decimal number, a quick grep over the WebCore source code shows that we prefer %d.

> Source/WebCore/html/track/TrackBase.cpp:63
> +    // If the value being assigned to this attribute is not an empty string
> +    // or a BCP 47 language tag[BCP47], then abort these steps.

This comment states what the code does. A good comment explains "why" the code does is operations. Either explain why we need to do this (if it is not self-explanatory) or please remove this comment.

> Source/WebCore/html/track/TrackBase.cpp:75
> +    if (!language.isEmpty() && !isValidBCP47LanguageTag(language)) {
> +        String message;
> +        if (language.contains('\0'))
> +            message = WTF::ASCIILiteral("The language contains a null character and is not a valid BCP 47 language tag.");
> +        else {
> +            StringBuilder stringBuilder;
> +            stringBuilder.appendLiteral("The language '");
> +            stringBuilder.append(language);
> +            stringBuilder.appendLiteral("' is not a valid BCP 47 language tag.");
> +            message = stringBuilder.toString();
> +        }
> +        element()->document().addConsoleMessage(MessageSource::Rendering, MessageLevel::Warning, message);

Do other browsers emit a warning message for an invalid language? Neither <https://html.spec.whatwg.org/multipage/embedded-content.html#attr-track-srclang> nor <http://www.w3.org/TR/2016/PR-media-source-20161004/#sourcebuffer-init-segment-received> state that we need to emit a warning for an invalid language.

> Source/WebCore/html/track/TrackBase.cpp:79
> +    m_language = language;

<http://www.w3.org/TR/2016/PR-media-source-20161004/#sourcebuffer-init-segment-received> seems to imply that we should take language to be the empty string when the language string is not a valid BCP 47 identifier.

> LayoutTests/media/only-bcp47-language-tags-accepted-as-valid-expected.txt:155
> +EXPECTED (videoTrack.language == 'invalid-char-') OK

Is this what other browser do? According to <https://html.spec.whatwg.org/multipage/embedded-content.html#dom-videotrack-language> we should return an empty string for the track language:
[[
The AudioTrack.language and VideoTrack.language attributes must return the BCP 47 language tag of the language of the track, if it has one, or the empty string otherwise. If the user agent is not able to express that language as a BCP 47 language tag (for example because the language information in the media resource's format is a free-form string without a defined interpretation), then the method must return the empty string, as if the track had no language. [INBAND]
]]

> LayoutTests/media/only-bcp47-language-tags-accepted-as-valid.html:55
> +                    videoTrack.language = notOverwritten;

Would it make sense to test AudioTracks?

[Daniel Bates]

Comment on attachment 297503 [details]
Patch

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

>> Source/WebCore/html/track/TrackBase.cpp:63
>> +    // or a BCP 47 language tag[BCP47], then abort these steps.
> 
> This comment states what the code does. A good comment explains "why" the code does is operations. Either explain why we need to do this (if it is not self-explanatory) or please remove this comment.

I know that you did not write this comment and though this comment did not reference a spec. the formatting, context lines, and the presence of a step numbers served as visual cues that this comment likely came from a spec. This comment comes from the an early revision of the Media Source Extensions spec. When you moved this comment you removed the leading sectional information and the step number, "10.1 language, on setting:\\1.", that served as visual cues. If we feel that there is value in annotating our code with the relevant spec legalese then we should reproduce such legalese verbatim. Even better we should update the comment to reference a permalink or a URL to the spec and mention the date so that a reader will know where the legalese came from and the version of the spec we implemented.

> Source/WebCore/html/track/VideoTrack.cpp:-205
> -    // 10.1 language, on setting:

Please do not remove this.

> Source/WebCore/html/track/VideoTrack.cpp:-208
> -    // 1. If the value being assigned to this attribute is not an empty string or a BCP 47 language
> -    // tag[BCP47], then abort these steps.
> -    // FIXME(123926): Validate the BCP47-ness of langague.

It seems weird that we are moving the validation logic out of this function that implements the spec. steps for processing a language. If the correct thing to do is to move the validation outside of this function then we should add a comment here that explains where the validation logic is located and even better explain why it could not be done here.

> Source/WebCore/html/track/VideoTrack.cpp:207
> -    // 3. If the sourceBuffer attribute on this track is not null, then queue a task to fire a simple
> +    // If the sourceBuffer attribute on this track is not null, then queue a task to fire a simple

Please do not remove the leading number. It serves as a visual cue that this line came from a spec. We update the top-most comment in this function to reference the spec.

> Source/WebCore/html/track/VideoTrack.cpp:212
> +    // Queue a task to fire a simple event named change at the VideoTrackList object referenced by

Ditto.

> Source/WebCore/platform/Language.cpp:204
> +bool isValidBCP47LanguageTag(const String& languageTag)
> +{
> +    if (languageTag.isEmpty())
> +        return false;
> +
> +    // https://tools.ietf.org/html/bcp47#section-2.1
> +    for (unsigned i = 0; i < languageTag.length(); ++i) {
> +        UChar c = languageTag[i];
> +        if (isASCIIAlphanumeric(c) || c == '-')
> +            continue;
> +        return false;
> +    }
> +    return true;
> +}

There is exactly one caller of this function in TrackBase::setLanguage(). Unless you plan to make use of this function in a subsequent bug/patch then I suggest we make this a static non-member, non-friend file function in file Source/WebCore/html/track/TrackBase.cpp.

[John Wilander]

(In reply to comment #24)
> Comment on attachment 297503 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=297503&action=review
> 
> > Source/WebCore/html/HTMLMediaElement.cpp:3777
> > +        LOG(Media, "HTMLMediaElement::configureTextTrackGroup(%p) -  '%s' track with language '%s' and BCP 47 language '%s' has score %i", this, textTrack->kindKeyword().string().utf8().data(), textTrack->language().string().utf8().data(), textTrack->validBCP47Language().string().utf8().data(), trackScore);
> 
> Nit: Although the format specifier %i is equivalent to %d for formatting a
> signed decimal number, a quick grep over the WebCore source code shows that
> we prefer %d.

Thanks. I'll change it as part of my patch.

> > Source/WebCore/html/track/TrackBase.cpp:63
> > +    // If the value being assigned to this attribute is not an empty string
> > +    // or a BCP 47 language tag[BCP47], then abort these steps.
> 
> This comment states what the code does. A good comment explains "why" the
> code does is operations. Either explain why we need to do this (if it is not
> self-explanatory) or please remove this comment.

Will comment on this in my reply to your follow-up.

> > Source/WebCore/html/track/TrackBase.cpp:75
> > +    if (!language.isEmpty() && !isValidBCP47LanguageTag(language)) {
> > +        String message;
> > +        if (language.contains('\0'))
> > +            message = WTF::ASCIILiteral("The language contains a null character and is not a valid BCP 47 language tag.");
> > +        else {
> > +            StringBuilder stringBuilder;
> > +            stringBuilder.appendLiteral("The language '");
> > +            stringBuilder.append(language);
> > +            stringBuilder.appendLiteral("' is not a valid BCP 47 language tag.");
> > +            message = stringBuilder.toString();
> > +        }
> > +        element()->document().addConsoleMessage(MessageSource::Rendering, MessageLevel::Warning, message);
> 
> Do other browsers emit a warning message for an invalid language? Neither
> <https://html.spec.whatwg.org/multipage/embedded-content.html#attr-track-
> srclang> nor
> <http://www.w3.org/TR/2016/PR-media-source-20161004/#sourcebuffer-init-
> segment-received> state that we need to emit a warning for an invalid
> language.

No, not that we know of we decided to do this because of two things:
1. We should help developers debug their code since with this change we will no longer accept non-compliant language tags internally. Without a warning there is no way for developers to understand what went wrong when their language setting is not picked up.
2. It allows for layout testing.

> > Source/WebCore/html/track/TrackBase.cpp:79
> > +    m_language = language;
> 
> <http://www.w3.org/TR/2016/PR-media-source-20161004/#sourcebuffer-init-
> segment-received> seems to imply that we should take language to be the
> empty string when the language string is not a valid BCP 47 identifier.

Are you saying we should drop the existing language setting (for internal use) if the page sets an invalid language? I actually implemented it that way first with just a boolean for whether m_language was valid or not.

If you mean language setting exposed to the page, see below.

> > LayoutTests/media/only-bcp47-language-tags-accepted-as-valid-expected.txt:155
> > +EXPECTED (videoTrack.language == 'invalid-char-') OK
> 
> Is this what other browser do? According to
> <https://html.spec.whatwg.org/multipage/embedded-content.html#dom-videotrack-
> language> we should return an empty string for the track language:
> [[
> The AudioTrack.language and VideoTrack.language attributes must return the
> BCP 47 language tag of the language of the track, if it has one, or the
> empty string otherwise. If the user agent is not able to express that
> language as a BCP 47 language tag (for example because the language
> information in the media resource's format is a free-form string without a
> defined interpretation), then the method must return the empty string, as if
> the track had no language. [INBAND]
> ]]

The discussion in my platform test pull request <https://github.com/w3c/web-platform-tests/pull/4359> ended with an acknowledgment that the spec is confusing but that BCP 47 compliance "is an authoring requirement, it does not affect user agents." We should just reflect to the page whatever the web developer decided to set. Thus the change only validates for internal purposes.

> > LayoutTests/media/only-bcp47-language-tags-accepted-as-valid.html:55
> > +                    videoTrack.language = notOverwritten;
> 
> Would it make sense to test AudioTracks?

We deliberately left AudioTrack out since it does not override TrackBase::setLanguage() and that was to be implemented in a follow-up patch by Jer. But since we moved validation to TrackBase::setLanguage() the validation will work for AudioTracks too and we should test it. Thanks!

[John Wilander]

(In reply to comment #25)
> Comment on attachment 297503 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=297503&action=review
> 
> >> Source/WebCore/html/track/TrackBase.cpp:63
> >> +    // or a BCP 47 language tag[BCP47], then abort these steps.
> > 
> > This comment states what the code does. A good comment explains "why" the code does is operations. Either explain why we need to do this (if it is not self-explanatory) or please remove this comment.
> 
> I know that you did not write this comment and though this comment did not
> reference a spec. the formatting, context lines, and the presence of a step
> numbers served as visual cues that this comment likely came from a spec.
> This comment comes from the an early revision of the Media Source Extensions
> spec. When you moved this comment you removed the leading sectional
> information and the step number, "10.1 language, on setting:\\1.", that
> served as visual cues. If we feel that there is value in annotating our code
> with the relevant spec legalese then we should reproduce such legalese
> verbatim. Even better we should update the comment to reference a permalink
> or a URL to the spec and mention the date so that a reader will know where
> the legalese came from and the version of the spec we implemented.

Got it. I'll restructure the comments and restore the proper spec quote.

> > Source/WebCore/html/track/VideoTrack.cpp:-205
> > -    // 10.1 language, on setting:
> 
> Please do not remove this.

Will fix.

> > Source/WebCore/html/track/VideoTrack.cpp:-208
> > -    // 1. If the value being assigned to this attribute is not an empty string or a BCP 47 language
> > -    // tag[BCP47], then abort these steps.
> > -    // FIXME(123926): Validate the BCP47-ness of langague.
> 
> It seems weird that we are moving the validation logic out of this function
> that implements the spec. steps for processing a language. If the correct
> thing to do is to move the validation outside of this function then we
> should add a comment here that explains where the validation logic is
> located and even better explain why it could not be done here.

I will add a comment.

> > Source/WebCore/html/track/VideoTrack.cpp:207
> > -    // 3. If the sourceBuffer attribute on this track is not null, then queue a task to fire a simple
> > +    // If the sourceBuffer attribute on this track is not null, then queue a task to fire a simple
> 
> Please do not remove the leading number. It serves as a visual cue that this
> line came from a spec. We update the top-most comment in this function to
> reference the spec.

Will fix.

> > Source/WebCore/html/track/VideoTrack.cpp:212
> > +    // Queue a task to fire a simple event named change at the VideoTrackList object referenced by
> 
> Ditto.

Will fix.

> > Source/WebCore/platform/Language.cpp:204
> > +bool isValidBCP47LanguageTag(const String& languageTag)
> > +{
> > +    if (languageTag.isEmpty())
> > +        return false;
> > +
> > +    // https://tools.ietf.org/html/bcp47#section-2.1
> > +    for (unsigned i = 0; i < languageTag.length(); ++i) {
> > +        UChar c = languageTag[i];
> > +        if (isASCIIAlphanumeric(c) || c == '-')
> > +            continue;
> > +        return false;
> > +    }
> > +    return true;
> > +}
> 
> There is exactly one caller of this function in TrackBase::setLanguage().
> Unless you plan to make use of this function in a subsequent bug/patch then
> I suggest we make this a static non-member, non-friend file function in file
> Source/WebCore/html/track/TrackBase.cpp.

The plan is to reuse this in HTTP header validation of Accept-Language and Content-language. But I don't know when that will happen. Would you prefer this to be a static non-member for now and then open it up when needed?

[Daniel Bates]

(In reply to comment #26)
> > > Source/WebCore/html/track/TrackBase.cpp:75
> > > +    if (!language.isEmpty() && !isValidBCP47LanguageTag(language)) {
> > > +        String message;
> > > +        if (language.contains('\0'))
> > > +            message = WTF::ASCIILiteral("The language contains a null character and is not a valid BCP 47 language tag.");
> > > +        else {
> > > +            StringBuilder stringBuilder;
> > > +            stringBuilder.appendLiteral("The language '");
> > > +            stringBuilder.append(language);
> > > +            stringBuilder.appendLiteral("' is not a valid BCP 47 language tag.");
> > > +            message = stringBuilder.toString();
> > > +        }
> > > +        element()->document().addConsoleMessage(MessageSource::Rendering, MessageLevel::Warning, message);
> > 
> > Do other browsers emit a warning message for an invalid language? Neither
> > <https://html.spec.whatwg.org/multipage/embedded-content.html#attr-track-
> > srclang> nor
> > <http://www.w3.org/TR/2016/PR-media-source-20161004/#sourcebuffer-init-
> > segment-received> state that we need to emit a warning for an invalid
> > language.
> 
> No, not that we know of we decided to do this because of two things:
> 1. We should help developers debug their code since with this change we will
> no longer accept non-compliant language tags internally. Without a warning
> there is no way for developers to understand what went wrong when their
> language setting is not picked up.
> 2. It allows for layout testing.
> 

OK. Is it necessary to include the language or whether it includes a null character in the error message? Can we take advantage of the facts that the track.srclang/texttrack.language attribute represents a single BCP 47 identifier (is this true?) and that a console message has line information to emit a generic error message? I also suggest that the error message explain how WebKit is handling this malformed language. (Are we ignoring the value of track.srclang/texttrack.language? What does it mean to ignore the text track language?)

[Daniel Bates]

(In reply to comment #26)
> > > Source/WebCore/html/track/TrackBase.cpp:79
> > > +    m_language = language;
> > 
> > <http://www.w3.org/TR/2016/PR-media-source-20161004/#sourcebuffer-init-
> > segment-received> seems to imply that we should take language to be the
> > empty string when the language string is not a valid BCP 47 identifier.
> 
> Are you saying we should drop the existing language setting (for internal
> use) if the page sets an invalid language? 

Disregard my comment as it does not seem applicable to this patch.

[Daniel Bates]

(In reply to comment #27)
> > > Source/WebCore/platform/Language.cpp:204
> > > +bool isValidBCP47LanguageTag(const String& languageTag)
> > > +{
> > > +    if (languageTag.isEmpty())
> > > +        return false;
> > > +
> > > +    // https://tools.ietf.org/html/bcp47#section-2.1
> > > +    for (unsigned i = 0; i < languageTag.length(); ++i) {
> > > +        UChar c = languageTag[i];
> > > +        if (isASCIIAlphanumeric(c) || c == '-')
> > > +            continue;
> > > +        return false;
> > > +    }
> > > +    return true;
> > > +}
> > 
> > There is exactly one caller of this function in TrackBase::setLanguage().
> > Unless you plan to make use of this function in a subsequent bug/patch then
> > I suggest we make this a static non-member, non-friend file function in file
> > Source/WebCore/html/track/TrackBase.cpp.
> 
> The plan is to reuse this in HTTP header validation of Accept-Language and
> Content-language. But I don't know when that will happen. Would you prefer
> this to be a static non-member for now and then open it up when needed?

Are you planing to reuse this the HTTP header validation logic in a patch in the near future (as in the next day or two)? If not, then I suggest that we make this a static non-member, non-friend file function in file Source/WebCore/html/track/TrackBase.cpp. When we come around to update the HTTP header validation logic then we can move this function to an appropriate place (maybe this file) so that it can be shared between the text track machinery and the HTTP header validation logic.

[John Wilander]

Created attachment 297693 [details]
Patch

[John Wilander]

Created attachment 297695 [details]
Patch

[John Wilander]

The GTK build was complaining about ambiguity in AtomicString::contains() for the call language.contains('\0').

[Build Bot]

Comment on attachment 297695 [details]
Patch

Attachment 297695 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2772708

New failing tests:
imported/w3c/web-platform-tests/html/dom/reflection-embedded.html
media/only-bcp47-language-tags-accepted-as-valid.html

[Build Bot]

Created attachment 297701 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[John Wilander]

Created attachment 297978 [details]
Patch

[Jer Noble]

Comment on attachment 297978 [details]
Patch

r=me. Nice job!

[John Wilander]

Thanks! Committing. I'll update it if Alex has further comments (we all discussed the rather complex validation logic).

[WebKit Commit Bot]

Comment on attachment 297978 [details]
Patch

Clearing flags on attachment: 297978

Committed r210280: <http://trac.webkit.org/changeset/210280>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35058112>