Bug 164538 – [WK2] autocorrect and autocapitalize attributes do not work in contenteditable elements

• Component: HTML Editing
• Status: RESOLVED FIXED

[Wenson Hsieh]

1. Go to whsieh.github.io/examples/contenteditable on an iOS device
2. Type in the contenteditable divs directly underneath the "autocapitalize disabled" and "autocorrect disabled" headings
3. Observe that autocapitalization and autocorrect are enabled, even though their attributes are set to "none" and "off", respectively.

[Wenson Hsieh]

<rdar://problem/8418711>

[Wenson Hsieh]

Created attachment 294212 [details]
Patch

[Ryosuke Niwa]

Note that these are non-standard attributes only supposed by WebKit and Blink.

[Build Bot]

Comment on attachment 294212 [details]
Patch

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

New failing tests:
storage/domstorage/sessionstorage/blocked-file-access.html

[Build Bot]

Created attachment 294341 [details]
Archive of layout-test-results from ews113 for mac-yosemite

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

[Joseph Pecoraro]

Comment on attachment 294212 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Adds autocorrect() and autocapitalizeType() to HTMLElements. These check the `autocorrect` and `autocapitalize`

Should we also move the IDL attributes to HTMLElement so that users can use elem.autocapitalize / elem.autocorrect getters/setters on any element?

It seems weird to me that we would support contentEditable / spellcheck IDL attributes on any element, and effectively support autocorrect/autocapitalize, but the only way to toggle them dynamically in script would be with elem.setAttribute instead of normal getters/setters (which already exist on form controls, like <input>).

> Tools/WebKitTestRunner/ios/UIKitSPI.h:62
> +@interface UIKeyboardImpl : UIView
> ++ (UIKeyboardImpl *)sharedInstance;
> +- (void)removeAllDynamicDictionaries;
> +@end

Any reason we are using the UIKeyboardImpl instance method and not the UIKeyboard class method?

> LayoutTests/ChangeLog:15
> +        * fast/events/ios/contenteditable-autocapitalize-none-expected.txt: Added.
> +        * fast/events/ios/contenteditable-autocapitalize-none.html: Added.
> +        * fast/events/ios/contenteditable-autocorrect-off-expected.txt: Added.
> +        * fast/events/ios/contenteditable-autocorrect-off.html: Added.

Can we also have tests for the other cases? Most notably autocapitalize-sentences / autocapitalize-characters and autocorrect-on?

[Wenson Hsieh]

Comment on attachment 294212 [details]
Patch

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

>> Source/WebCore/ChangeLog:9
>> +        Adds autocorrect() and autocapitalizeType() to HTMLElements. These check the `autocorrect` and `autocapitalize`
> 
> Should we also move the IDL attributes to HTMLElement so that users can use elem.autocapitalize / elem.autocorrect getters/setters on any element?
> 
> It seems weird to me that we would support contentEditable / spellcheck IDL attributes on any element, and effectively support autocorrect/autocapitalize, but the only way to toggle them dynamically in script would be with elem.setAttribute instead of normal getters/setters (which already exist on form controls, like <input>).

Sounds good -- I'll add IDL attributes for autocapitalize and autocorrect.

>> Tools/WebKitTestRunner/ios/UIKitSPI.h:62
>> +@end
> 
> Any reason we are using the UIKeyboardImpl instance method and not the UIKeyboard class method?

Oh, I didn't see +[UIKeyboard removeAllDynamicDictionaries]! We should definitely be using that instead -- fixed.

>> LayoutTests/ChangeLog:15
>> +        * fast/events/ios/contenteditable-autocorrect-off.html: Added.
> 
> Can we also have tests for the other cases? Most notably autocapitalize-sentences / autocapitalize-characters and autocorrect-on?

Sounds good -- I'll add those as well.

[Ryosuke Niwa]

If we're adding this feature on Mac, I'd suggest that we propose it on Web Platform WG or editing TF.

[Wenson Hsieh]

(In reply to comment #8)
> If we're adding this feature on Mac, I'd suggest that we propose it on Web
> Platform WG or editing TF.

These attributes are guarded by IOS_AUTOCORRECT_AND_AUTOCAPITALIZE, which appears to be only defined as 1 for iOS.

[Ryosuke Niwa]

(In reply to comment #9)
> (In reply to comment #8)
> > If we're adding this feature on Mac, I'd suggest that we propose it on Web
> > Platform WG or editing TF.
> 
> These attributes are guarded by IOS_AUTOCORRECT_AND_AUTOCAPITALIZE, which
> appears to be only defined as 1 for iOS.

Oh, you're just fixing this for iOS WK2.

[Wenson Hsieh]

Created attachment 294428 [details]
Patch

[Wenson Hsieh]

Created attachment 294437 [details]
Fixed bad rebase in LayoutTests/ChangeLog

[Joseph Pecoraro]

Comment on attachment 294437 [details]
Fixed bad rebase in LayoutTests/ChangeLog

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

Looks good to me!

> Source/WebCore/html/HTMLElement.h:26
>  #include "StyledElement.h"
> +#if ENABLE(IOS_AUTOCORRECT_AND_AUTOCAPITALIZE)

Style: You should have a blank line before the #if line.

> LayoutTests/fast/events/ios/contenteditable-autocapitalize-expected.txt:5
> +With autocapitalize: none, the output is: "to"
> +With autocapitalize: sentences, the output is: "To"
> +With autocapitalize: characters, the output is: "TO"

You could even test "words" but it requires a slightly longer test string. Either way this is a big improvement!

> LayoutTests/fast/events/ios/contenteditable-autocorrect-expected.txt:3
> +With autocorrect off, the result is: "Ti "

What is the special character here? Is it significant? Should we add <meta charset="utf-8"> to the test?

[Wenson Hsieh]

Comment on attachment 294437 [details]
Fixed bad rebase in LayoutTests/ChangeLog

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

>> Source/WebCore/html/HTMLElement.h:26
>> +#if ENABLE(IOS_AUTOCORRECT_AND_AUTOCAPITALIZE)
> 
> Style: You should have a blank line before the #if line.

Got it -- added a newline.

>> LayoutTests/fast/events/ios/contenteditable-autocorrect-expected.txt:3
>> +With autocorrect off, the result is: "Ti "
> 
> What is the special character here? Is it significant? Should we add <meta charset="utf-8"> to the test?

Oh, interesting -- it looks like we insert a non-breaking space when autocorrecting! I'll add <meta charset="utf-8"> -- that should make this test more robust if we decide to change things in the future.

[Ryosuke Niwa]

Comment on attachment 294437 [details]
Fixed bad rebase in LayoutTests/ChangeLog

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

> Source/WebCore/html/HTMLElement.cpp:1070
> +bool HTMLElement::autocorrect() const

This function should be called shouldAutocorrect instead?

> Source/WebCore/html/HTMLElement.cpp:1090
> +void HTMLElement::setAutocorrect(bool autocorrect)
> +{
> +    setAttributeWithoutSynchronization(autocorrectAttr, autocorrect ? AtomicString("on", AtomicString::ConstructFromLiteral) : AtomicString("off", AtomicString::ConstructFromLiteral));
> +}

Please put this right beneath autocorrect().

> Source/WebCore/html/HTMLElement.idl:52
> +    [Conditional=IOS_AUTOCORRECT_AND_AUTOCAPITALIZE] attribute boolean autocorrect;
> +    [Conditional=IOS_AUTOCORRECT_AND_AUTOCAPITALIZE, TreatNullAs=EmptyString] attribute DOMString autocapitalize;
> +

This would make all HTMLElement have these properties instead of just HTMLInputElement.
Adding any property on HTMLElement can break content that relies on those properties to not exist.
So at minimum, we should propose this in the standards.

> LayoutTests/fast/events/ios/contenteditable-autocapitalize.html:8
> +        let write = s => output.innerHTML += s + "<br>";

Please don't use one letter variable like s.

> LayoutTests/fast/events/ios/contenteditable-autocapitalize.html:62
> +            switch (progress) {
> +            case 2:
> +                outputEditableAndSetAutocapitalize("sentences");
> +                break;
> +            case 3:
> +                testRunner.runUIScript(getUIScript(), incrementProgress);
> +                break;
> +            case 5:
> +                outputEditableAndSetAutocapitalize("characters");
> +                break;
> +            case 6:
> +                testRunner.runUIScript(getUIScript(), incrementProgress);
> +                break;
> +            case 8:
> +                outputEditableAndSetAutocapitalize(null);
> +                break;
> +            case 9:
> +                testRunner.notifyDone();
> +                break;
> +            }

Instead of having a giant switch, why can't we just use an array of functions that we loop over?
Also, where did 4 & 7 go?

> LayoutTests/fast/events/ios/contenteditable-autocorrect.html:57
> +            case 3:
> +                write(`With autocorrect off, the result is: "${editable.textContent}"`);
> +                editable.textContent = "";
> +                editable.autocorrect = "on";
> +                editable.blur();
> +                break;
> +            case 4:
> +                write(`Trying again with autocorrect on:`);
> +                testRunner.runUIScript(getUIScript(), incrementProgress);
> +                break;
> +            case 8:
> +                write(`With autocorrect on, the result is: "${editable.textContent}"`);
> +                editable.textContent = "";
> +                editable.blur();
> +                break;
> +            case 9:
> +                testRunner.notifyDone();
> +                break;

Again, numbers getting skipped is very cryptic.

[Wenson Hsieh]

Comment on attachment 294437 [details]
Fixed bad rebase in LayoutTests/ChangeLog

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

>> LayoutTests/fast/events/ios/contenteditable-autocapitalize.html:8
>> +        let write = s => output.innerHTML += s + "<br>";
> 
> Please don't use one letter variable like s.

Ok -- I'll rename this.

>> LayoutTests/fast/events/ios/contenteditable-autocapitalize.html:62
>> +            }
> 
> Instead of having a giant switch, why can't we just use an array of functions that we loop over?
> Also, where did 4 & 7 go?

That's a good idea! I'll change it to do that. 1, 4 and 7 are missing from the switch case because no action needs to be taken at those progress counts to move the test forward. This is because at those points in time, I'm waiting for the rest of the input to happen (for instance, when progress == 4, we've typed 't', and are waiting for the 'o' to arrive before logging the output and continuing with the next script).

>> LayoutTests/fast/events/ios/contenteditable-autocorrect.html:57
>> +                break;
> 
> Again, numbers getting skipped is very cryptic.

I'll employ the "array of progress handlers" tactic here.

[Wenson Hsieh]

Comment on attachment 294437 [details]
Fixed bad rebase in LayoutTests/ChangeLog

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

>> Source/WebCore/html/HTMLElement.cpp:1070
>> +bool HTMLElement::autocorrect() const
> 
> This function should be called shouldAutocorrect instead?

Sounds good -- renamed.

>> Source/WebCore/html/HTMLElement.cpp:1090
>> +}
> 
> Please put this right beneath autocorrect().

Done!

>> Source/WebCore/html/HTMLElement.idl:52
>> +
> 
> This would make all HTMLElement have these properties instead of just HTMLInputElement.
> Adding any property on HTMLElement can break content that relies on those properties to not exist.
> So at minimum, we should propose this in the standards.

Ok. I will propose this.

[Wenson Hsieh]

Created attachment 294468 [details]
Refactor layout tests + address other feedback

[Ryosuke Niwa]

Comment on attachment 294468 [details]
Refactor layout tests + address other feedback

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

> Source/WebCore/html/HTMLFormControlElement.h:96
> +    WEBCORE_EXPORT WebAutocapitalizeType autocapitalizeType() const final;

Web* type shouldn't really be used in WebCore. We should add a new enum to be used in WebCore
which then gets converted to Web* in WebKit layer.

> LayoutTests/fast/events/ios/contenteditable-autocapitalize.html:9
> +        function getUIScript() {

This doesn't really need to be a function.
I think it's cleaner to have this template literal inside runUIScript where you call testRunner.runUIScript.

> LayoutTests/fast/events/ios/contenteditable-autocapitalize.html:79
> +            waitForNumberOfInputEvents(2).then(function() {
> +                outputEditableAndSetAutocapitalize("sentences");
> +            });

This still has a problem that it's unclear why we have to wait for 2.

A better approach is to wrap the whole process of running UI script and waiting for the events in a promise.
e.g.

let remainingInputEventCount = 0;
let resolveInputEvent = null;
function handleInput() {
    remainingInputEventCount--;
    if (resolveInputEvent && !remainingInputEventCount)
        resolveInputEvent();
}

function runUIScriptAndWait(inputEventCount = 2) {
    return Promise.all([new Promise((resolve) => {
        remainingInputEventCount = inputEventCount;
        resolveInputEvent = resolve;
    }), runUIScript()]);
}

function runTest() {
    runUIScriptAndWait().then(() => outputEditableAndSetAutocapitalize("sentences"))
        .then(() => runUIScriptAndWait()).then(() => outputEditableAndSetAutocapitalize("characters"))
        .then(() => runUIScriptAndWait()).then(() => outputEditableAndSetAutocapitalize(null));
}

[Wenson Hsieh]

Comment on attachment 294468 [details]
Refactor layout tests + address other feedback

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

>> Source/WebCore/html/HTMLFormControlElement.h:96
>> +    WEBCORE_EXPORT WebAutocapitalizeType autocapitalizeType() const final;
> 
> Web* type shouldn't really be used in WebCore. We should add a new enum to be used in WebCore
> which then gets converted to Web* in WebKit layer.

I agree that "Web-" prefixed enums don't really belong in WebCore. However, it's not actually clear to me why there needs to be a Web- prefix here in the first place, considering we use WebCore classes in other places in WebKit and WebKit2 as well (e.g. various types of HTMLElements). Is there a reason this can't just be AutocapitalizeType?

Either way, I think this refactoring would make more sense as a separate patch.

>> LayoutTests/fast/events/ios/contenteditable-autocapitalize.html:9
>> +        function getUIScript() {
> 
> This doesn't really need to be a function.
> I think it's cleaner to have this template literal inside runUIScript where you call testRunner.runUIScript.

Good point -- folded into runUIScript.

>> LayoutTests/fast/events/ios/contenteditable-autocapitalize.html:79
>> +            });
> 
> This still has a problem that it's unclear why we have to wait for 2.
> 
> A better approach is to wrap the whole process of running UI script and waiting for the events in a promise.
> e.g.
> 
> let remainingInputEventCount = 0;
> let resolveInputEvent = null;
> function handleInput() {
>     remainingInputEventCount--;
>     if (resolveInputEvent && !remainingInputEventCount)
>         resolveInputEvent();
> }
> 
> function runUIScriptAndWait(inputEventCount = 2) {
>     return Promise.all([new Promise((resolve) => {
>         remainingInputEventCount = inputEventCount;
>         resolveInputEvent = resolve;
>     }), runUIScript()]);
> }
> 
> function runTest() {
>     runUIScriptAndWait().then(() => outputEditableAndSetAutocapitalize("sentences"))
>         .then(() => runUIScriptAndWait()).then(() => outputEditableAndSetAutocapitalize("characters"))
>         .then(() => runUIScriptAndWait()).then(() => outputEditableAndSetAutocapitalize(null));
> }

runUIScript will never resolve before the expected input event count promise resolves, since the UI script completes when the keyboard is done dismissing. This only happens after the expected input events have been observed and outputEditableAndSetAutocapitalize has been invoked, so the approach above causes us to time out. I can try to refactor the test here so that we can follow a runUIScriptAndWait().then()... pattern.

[Wenson Hsieh]

Created attachment 294480 [details]
Tweaked layout tests

[Ryosuke Niwa]

(In reply to comment #20)
> Comment on attachment 294468 [details]
> Refactor layout tests + address other feedback
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=294468&action=review
> 
> >> Source/WebCore/html/HTMLFormControlElement.h:96
> >> +    WEBCORE_EXPORT WebAutocapitalizeType autocapitalizeType() const final;
> > 
> > Web* type shouldn't really be used in WebCore. We should add a new enum to be used in WebCore
> > which then gets converted to Web* in WebKit layer.
> 
> I agree that "Web-" prefixed enums don't really belong in WebCore. However,
> it's not actually clear to me why there needs to be a Web- prefix here in
> the first place, considering we use WebCore classes in other places in
> WebKit and WebKit2 as well (e.g. various types of HTMLElements). Is there a
> reason this can't just be AutocapitalizeType?

If Web- prefixes are used outside WebCore, then it needs to be separate enums from the ones used in WebCore for licensing reasons. WebCore is LGPL. WebKit/WebKit2 are BSD.

> >> LayoutTests/fast/events/ios/contenteditable-autocapitalize.html:9
> >> +        function getUIScript() {
> > 
> > This doesn't really need to be a function.
> > I think it's cleaner to have this template literal inside runUIScript where you call testRunner.runUIScript.
> 
> Good point -- folded into runUIScript.
> 
> >> LayoutTests/fast/events/ios/contenteditable-autocapitalize.html:79
> >> +            });
> > 
> > This still has a problem that it's unclear why we have to wait for 2.
> > 
> > A better approach is to wrap the whole process of running UI script and waiting for the events in a promise.
> > e.g.
> > 
> > let remainingInputEventCount = 0;
> > let resolveInputEvent = null;
> > function handleInput() {
> >     remainingInputEventCount--;
> >     if (resolveInputEvent && !remainingInputEventCount)
> >         resolveInputEvent();
> > }
> > 
> > function runUIScriptAndWait(inputEventCount = 2) {
> >     return Promise.all([new Promise((resolve) => {
> >         remainingInputEventCount = inputEventCount;
> >         resolveInputEvent = resolve;
> >     }), runUIScript()]);
> > }
> > 
> > function runTest() {
> >     runUIScriptAndWait().then(() => outputEditableAndSetAutocapitalize("sentences"))
> >         .then(() => runUIScriptAndWait()).then(() => outputEditableAndSetAutocapitalize("characters"))
> >         .then(() => runUIScriptAndWait()).then(() => outputEditableAndSetAutocapitalize(null));
> > }
> 
> runUIScript will never resolve before the expected input event count promise
> resolves, since the UI script completes when the keyboard is done
> dismissing. This only happens after the expected input events have been
> observed and outputEditableAndSetAutocapitalize has been invoked, so the
> approach above causes us to time out.

What is outputEditableAndSetAutocapitalize anything to do with UI scripts?  It is somehow updating some dates to which UI script responds?  It would be misleading to do that in a function called outputEditableAndSetAutocapitalize. We should probably separate that into a separate function which communicates back to the UI script.

[Wenson Hsieh]

(In reply to comment #22)
> (In reply to comment #20)
> > runUIScript will never resolve before the expected input event count promise
> > resolves, since the UI script completes when the keyboard is done
> > dismissing. This only happens after the expected input events have been
> > observed and outputEditableAndSetAutocapitalize has been invoked, so the
> > approach above causes us to time out.
> 
> What is outputEditableAndSetAutocapitalize anything to do with UI scripts? 
> It is somehow updating some dates to which UI script responds?  It would be
> misleading to do that in a function called
> outputEditableAndSetAutocapitalize. We should probably separate that into a
> separate function which communicates back to the UI script.

outputEditableAndSetAutocapitalize dismisses the keyboard by blurring the contenteditable. The UI script does not complete until the keyboard has dismissed. I've actually removed outputEditableAndSetAutocapitalize (and outputEditableAndSetAutocorrect) entirely and moved the logic into runUIScriptAndExpectInputEvents, so runTest() now looks something like:

runUIScriptAndExpectInputEvents(2, "sentences")
    .then(() => runUIScriptAndExpectInputEvents(2, "characters"))
    .then(() => runUIScriptAndExpectInputEvents(2, null))
    .then(() => testRunner.notifyDone());

[Wenson Hsieh]

Created attachment 294498 [details]
Remove WebAutocapitalizeType from WebCore

[Joseph Pecoraro]

Comment on attachment 294498 [details]
Remove WebAutocapitalizeType from WebCore

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

> Source/WebKit/mac/DOM/WebAutocapitalizeTypes.h:26
> +#pragma once

This normally is not needed in ObjC headers.

Speaking of the ObjC DOM APIs, should we move autocorrect and autocapitalize to DOMHTMLElement to match the IDL?

[Wenson Hsieh]

(In reply to comment #25)
> Comment on attachment 294498 [details]
> Remove WebAutocapitalizeType from WebCore
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=294498&action=review
> 
> > Source/WebKit/mac/DOM/WebAutocapitalizeTypes.h:26
> > +#pragma once
> 
> This normally is not needed in ObjC headers.

Good catch -- removed.

> 
> Speaking of the ObjC DOM APIs, should we move autocorrect and autocapitalize
> to DOMHTMLElement to match the IDL?

Yes, this seems reasonable. I'll make this change.

[Wenson Hsieh]

Created attachment 294510 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 294510 [details]
Patch

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

> Source/WebCore/html/Autocapitalize.h:39
> +typedef enum {
> +    AutocapitalizeTypeDefault,
> +    AutocapitalizeTypeNone,
> +    AutocapitalizeTypeWords,
> +    AutocapitalizeTypeSentences,
> +    AutocapitalizeTypeAllCharacters
> +} AutocapitalizeType;

Please use modern C++ style enum:
enum class AutocapitalizeType {
    Default,
...
};

> Source/WebCore/html/HTMLElement.h:27
> +#if ENABLE(IOS_AUTOCORRECT_AND_AUTOCAPITALIZE)
> +#include "Autocapitalize.h"
> +#endif

We typically put this if-def inside the header instead.

> Source/WebKit/mac/DOM/WebAutocapitalizeTypes.h:32
> +typedef enum {
> +    WebAutocapitalizeTypeDefault,
> +    WebAutocapitalizeTypeNone,
> +    WebAutocapitalizeTypeWords,
> +    WebAutocapitalizeTypeSentences,
> +    WebAutocapitalizeTypeAllCharacters
> +} WebAutocapitalizeType;

Please use modern enum syntax.
Even if you couldn't use enum class, please use:
enum WebAutocapitalizeType {
~
};
instead as C++ doesn't require typedef for enums.

> Source/WebKit2/Shared/AssistedNodeInformation.h:36
> +typedef enum {

Ditto about not using typedef.

I'm a bit confused here. I think you misunderstood what I said.
It's fine for WebCore header & enum to be used in WebKit1 / WebKit2 code.
What I mean is that WebKit1 / WebKit2 can't export those enums as API.
Since we're already converting these enums to UIKit types in WebKit1 / WebKit2,
we don't need a separate enum inside them.
Furthermore, relying on these two declarations to match is fragile.

[Ryosuke Niwa]

Comment on attachment 294510 [details]
Patch

r=me provided you get rid of enum declarations in WebKit & WebKit2.

[Wenson Hsieh]

(In reply to comment #29)
> Comment on attachment 294510 [details]
> Patch
> 
> r=me provided you get rid of enum declarations in WebKit & WebKit2.

Thanks!

I'll remove WKAutocapitalizeType. However, we still need a WebAutocapitalizeType declared, otherwise we'll break UIKit which uses WebKitLegacy's WebAutocapitalizeType. There doesn't seem to be a reason for UIKit to be doing this though, and I think we should change UIKit to not care about WebAutocapitalizeType (and just receive a UITextAutocapitalizationType from WebKitLegacy).

I will introduce a WebAutocapitalizeType, but make the conversion more robust using a helper function with a switch case.

[Wenson Hsieh]

Created attachment 294542 [details]
Patch for landing

[Wenson Hsieh]

Created attachment 294543 [details]
Patch for landing

[Wenson Hsieh]

Created attachment 294547 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 294547 [details]
Patch for landing

Rejecting attachment 294547 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 294547, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
fs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 208614 = b6b117a0e51cdf7c69db580820553ecb9c0eecbf
r208615 = f991ec35da7209373ed165c0906a2bc60495960e
r208616 = d7dac224a36de4546d35bbb4b568156ea2b82e4f
r208617 = 2fbddb6e7619c8ab572795f1d2513e9c3032cda7
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.webkit.org/results/2500173

[Wenson Hsieh]

Created attachment 294553 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 294553 [details]
Patch for landing

Clearing flags on attachment: 294553

Committed r208624: <http://trac.webkit.org/changeset/208624>