Bug 185375 - Add support for Intl NumberFormat formatToParts
Summary: Add support for Intl NumberFormat formatToParts
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Safari 11
Hardware: Unspecified Unspecified
: P2 Enhancement
Assignee: Andy VanWagoner
URL:
Keywords: InRadar
Depends on: 185502
Blocks:
  Show dependency treegraph
 
Reported: 2018-05-07 06:53 PDT by Robin Whittleton
Modified: 2019-03-26 00:51 PDT (History)
8 users (show)

See Also:


Attachments
Patch (69.47 KB, patch)
2018-05-10 08:58 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff
Patch (69.43 KB, patch)
2018-05-10 09:15 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-sierra (2.25 MB, application/zip)
2018-05-10 10:39 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-sierra-wk2 (2.72 MB, application/zip)
2018-05-10 10:44 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews114 for mac-sierra (3.01 MB, application/zip)
2018-05-10 11:07 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (2.31 MB, application/zip)
2018-05-10 11:11 PDT, Build Bot
no flags Details
Separate tests for formatToParts, disabled on macOS (71.30 KB, patch)
2018-05-10 16:30 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews126 for ios-simulator-wk2 (2.19 MB, application/zip)
2018-05-10 18:28 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews206 for win-future (12.74 MB, application/zip)
2018-05-10 19:20 PDT, Build Bot
no flags Details
Patch (72.36 KB, patch)
2018-05-10 21:00 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews201 for win-future (12.76 MB, application/zip)
2018-05-11 01:07 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews204 for win-future (12.81 MB, application/zip)
2018-05-11 03:07 PDT, Build Bot
no flags Details
Set test expectations properly (73.04 KB, patch)
2018-05-11 08:00 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff
Patch (74.34 KB, patch)
2018-05-11 10:00 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff
Patch (74.45 KB, patch)
2018-05-11 13:29 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff
rebased (73.73 KB, patch)
2018-05-14 10:25 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-sierra-wk2 (3.14 MB, application/zip)
2018-05-14 11:51 PDT, Build Bot
no flags Details
Patch (73.79 KB, patch)
2018-05-15 10:20 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff
rebased (73.79 KB, patch)
2018-05-16 12:58 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robin Whittleton 2018-05-07 06:53:16 PDT
I’d love to see formatToParts supported - at the moment we’re having to add a polyfill for Safari (and Edge, although native support is now there in Firefox and Chrome). This lets us use Intl’s numberFormat to produce currency strings with all the global display inconsistencies, but also lets us support target markets in customisations they want to do (e.g. use a custom currency marker or replace the decimals with a dash when it’s a round number).

MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/NumberFormat/formatToParts
ECMA 402 issue: https://github.com/tc39/ecma402/issues/30
Comment 1 Andy VanWagoner 2018-05-10 08:58:31 PDT
Created attachment 340092 [details]
Patch
Comment 2 Build Bot 2018-05-10 09:00:50 PDT
Attachment 340092 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Andy VanWagoner 2018-05-10 09:15:10 PDT
Created attachment 340094 [details]
Patch
Comment 4 Build Bot 2018-05-10 10:39:22 PDT
Comment on attachment 340094 [details]
Patch

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

New failing tests:
js/intl-numberformat.html
Comment 5 Build Bot 2018-05-10 10:39:23 PDT
Created attachment 340101 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 6 Build Bot 2018-05-10 10:44:18 PDT
Comment on attachment 340094 [details]
Patch

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

New failing tests:
js/intl-numberformat.html
Comment 7 Build Bot 2018-05-10 10:44:19 PDT
Created attachment 340102 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 8 Build Bot 2018-05-10 10:47:06 PDT
Comment on attachment 340094 [details]
Patch

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

New failing tests:
jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-no-llint
jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-dfg-eager-no-cjit
jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-ftl-eager-no-cjit
jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-no-ftl
jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-no-cjit
jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout
jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-ftl-no-cjit
apiTests
Comment 9 Build Bot 2018-05-10 11:07:46 PDT
Comment on attachment 340094 [details]
Patch

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

New failing tests:
js/intl-numberformat.html
Comment 10 Build Bot 2018-05-10 11:07:48 PDT
Created attachment 340108 [details]
Archive of layout-test-results from ews114 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 11 Build Bot 2018-05-10 11:11:08 PDT
Comment on attachment 340094 [details]
Patch

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

New failing tests:
js/intl-numberformat.html
Comment 12 Build Bot 2018-05-10 11:11:09 PDT
Created attachment 340110 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 13 Andy VanWagoner 2018-05-10 16:30:21 PDT
Created attachment 340145 [details]
Separate tests for formatToParts, disabled on macOS
Comment 14 Andy VanWagoner 2018-05-10 16:37:20 PDT
Comment on attachment 340145 [details]
Separate tests for formatToParts, disabled on macOS

Since this feature relies on ICU v59, expect it to fail in macOS for now.
Comment 15 Build Bot 2018-05-10 18:28:02 PDT
Comment on attachment 340145 [details]
Separate tests for formatToParts, disabled on macOS

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

New failing tests:
js/intl-numberformat-format-to-parts.html
Comment 16 Build Bot 2018-05-10 18:28:03 PDT
Created attachment 340155 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 17 Build Bot 2018-05-10 18:51:59 PDT
Comment on attachment 340145 [details]
Separate tests for formatToParts, disabled on macOS

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

New failing tests:
jsc-layout-tests.yaml/js/script-tests/intl-numberformat-format-to-parts.js.layout
jsc-layout-tests.yaml/js/script-tests/intl-numberformat-format-to-parts.js.layout-ftl-eager-no-cjit
jsc-layout-tests.yaml/js/script-tests/intl-numberformat-format-to-parts.js.layout-no-ftl
jsc-layout-tests.yaml/js/script-tests/intl-numberformat-format-to-parts.js.layout-no-llint
jsc-layout-tests.yaml/js/script-tests/intl-numberformat-format-to-parts.js.layout-no-cjit
jsc-layout-tests.yaml/js/script-tests/intl-numberformat-format-to-parts.js.layout-dfg-eager-no-cjit
jsc-layout-tests.yaml/js/script-tests/intl-numberformat-format-to-parts.js.layout-ftl-no-cjit
apiTests
Comment 18 Build Bot 2018-05-10 19:20:07 PDT
Comment on attachment 340145 [details]
Separate tests for formatToParts, disabled on macOS

Attachment 340145 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/7645496

New failing tests:
js/intl-numberformat-format-to-parts.html
Comment 19 Build Bot 2018-05-10 19:20:19 PDT
Created attachment 340158 [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
Comment 20 Andy VanWagoner 2018-05-10 21:00:37 PDT
Created attachment 340166 [details]
Patch
Comment 21 Build Bot 2018-05-11 01:07:43 PDT
Comment on attachment 340166 [details]
Patch

Attachment 340166 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/7648248

New failing tests:
js/intl-numberformat-format-to-parts.html
Comment 22 Build Bot 2018-05-11 01:07:54 PDT
Created attachment 340176 [details]
Archive of layout-test-results from ews201 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews201  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 23 Build Bot 2018-05-11 03:07:17 PDT
Comment on attachment 340166 [details]
Patch

Attachment 340166 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/7649039

New failing tests:
js/intl-numberformat-format-to-parts.html
Comment 24 Build Bot 2018-05-11 03:07:29 PDT
Created attachment 340183 [details]
Archive of layout-test-results from ews204 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 25 Andy VanWagoner 2018-05-11 08:00:39 PDT
Created attachment 340191 [details]
Set test expectations properly
Comment 26 Yusuke Suzuki 2018-05-11 09:02:55 PDT
Comment on attachment 340191 [details]
Set test expectations properly

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

> Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:628
> +        for (const auto &candidate : fields) {
> +            if (candidate.beginIndex <= currentIndex && currentIndex < candidate.endIndex && (!field.size() || candidate.size() < field.size()))
> +                field = candidate;
> +            if (currentIndex < candidate.beginIndex && candidate.beginIndex < nextStartIndex)
> +                nextStartIndex = candidate.beginIndex;
> +        }

I think if we sort this fields before iterating, we can iteratively accesses fields. In that case, time is O(NlogN), correct?

> Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:635
> +        parts->push(&exec, part);

Do not use `push()` since it is used for ArrayPush and it is observable to users (it uses [[Put]]).
Use putDirectIndex instead.

> Source/JavaScriptCore/runtime/IntlNumberFormat.h:34
> +#define JSC_ICU_HAS_FORMAT_DOUBLE_FOR_FIELDS (U_ICU_VERSION_MAJOR_NUM >= 59)

You can make this `#define HAVE_ICU_FORMAT_DOUBLE_FOR_FIELDS` and use it as `#if HAVE(ICU_FORMAT_DOUBLE_FOR_FIELDS)`.

> Source/JavaScriptCore/runtime/IntlNumberFormatPrototype.cpp:86
> +        JSFunction* formatToPartsFunction = JSFunction::create(vm, globalObject, 1, vm.propertyNames->formatToParts.string(), IntlNumberFormatPrototypeFuncFormatToParts);
> +        putDirectWithoutTransition(vm, vm.propertyNames->formatToParts, formatToPartsFunction, static_cast<unsigned>(PropertyAttribute::DontEnum));

Use JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION instead.

> Source/cmake/WebKitFeatures.cmake:128
> +    WEBKIT_OPTION_DEFINE(ENABLE_INTL_NUMBER_FORMAT_TO_PARTS "Toggle Intl.NumberFormat.prototype.formatToParts support" PRIVATE OFF)

Add this option to OptionsJSCOnly.cmake and enable it for JSCOnly. (See ENABLE_INTL_PLURAL_RULES in OptionsJSCOnly.cmake).
Comment 27 Yusuke Suzuki 2018-05-11 09:08:02 PDT
Comment on attachment 340191 [details]
Set test expectations properly

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

> Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:631
> +        auto value = jsString(&exec, resultString.substring(currentIndex, nextIndex - currentIndex));

Use `jsSubstring(VM*, const String&, unsigned offset, unsigned length)`.
Comment 28 Andy VanWagoner 2018-05-11 09:55:02 PDT
(In reply to Yusuke Suzuki from comment #26)
> Comment on attachment 340191 [details]
> Set test expectations properly
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=340191&action=review
> 
> > Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:628
> > +        for (const auto &candidate : fields) {
> > +            if (candidate.beginIndex <= currentIndex && currentIndex < candidate.endIndex && (!field.size() || candidate.size() < field.size()))
> > +                field = candidate;
> > +            if (currentIndex < candidate.beginIndex && candidate.beginIndex < nextStartIndex)
> > +                nextStartIndex = candidate.beginIndex;
> > +        }
> 
> I think if we sort this fields before iterating, we can iteratively accesses
> fields. In that case, time is O(NlogN), correct?

The fields are nested, so order doesn't trivialize this loop anyway. There is one integer field spanning all of the group separators and digits up to the decimal separator.

I could do a sort first which is O(NlogN) itself, but given the bounds of N (Number.MAX_NUMBER has ~100), I'm not sure it's worth the additional code complexity. The time here is dominated by unum_formatDoubleForFields.

> 
> > Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:635
> > +        parts->push(&exec, part);
> 
> Do not use `push()` since it is used for ArrayPush and it is observable to
> users (it uses [[Put]]).
> Use putDirectIndex instead.

Will do. `parts->putDirectIndex(&exec, parts->length(), part);` ?

> 
> > Source/JavaScriptCore/runtime/IntlNumberFormat.h:34
> > +#define JSC_ICU_HAS_FORMAT_DOUBLE_FOR_FIELDS (U_ICU_VERSION_MAJOR_NUM >= 59)
> 
> You can make this `#define HAVE_ICU_FORMAT_DOUBLE_FOR_FIELDS` and use it as
> `#if HAVE(ICU_FORMAT_DOUBLE_FOR_FIELDS)`.

That looks much nicer. Will do.

> 
> > Source/JavaScriptCore/runtime/IntlNumberFormatPrototype.cpp:86
> > +        JSFunction* formatToPartsFunction = JSFunction::create(vm, globalObject, 1, vm.propertyNames->formatToParts.string(), IntlNumberFormatPrototypeFuncFormatToParts);
> > +        putDirectWithoutTransition(vm, vm.propertyNames->formatToParts, formatToPartsFunction, static_cast<unsigned>(PropertyAttribute::DontEnum));
> 
> Use JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION instead.

Will do.

> 
> > Source/cmake/WebKitFeatures.cmake:128
> > +    WEBKIT_OPTION_DEFINE(ENABLE_INTL_NUMBER_FORMAT_TO_PARTS "Toggle Intl.NumberFormat.prototype.formatToParts support" PRIVATE OFF)
> 
> Add this option to OptionsJSCOnly.cmake and enable it for JSCOnly. (See
> ENABLE_INTL_PLURAL_RULES in OptionsJSCOnly.cmake).

Will do.
Comment 29 Andy VanWagoner 2018-05-11 10:00:58 PDT
Created attachment 340194 [details]
Patch
Comment 30 Yusuke Suzuki 2018-05-11 10:09:04 PDT
Comment on attachment 340191 [details]
Set test expectations properly

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

>>> Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:628
>>> +        }
>> 
>> I think if we sort this fields before iterating, we can iteratively accesses fields. In that case, time is O(NlogN), correct?
> 
> The fields are nested, so order doesn't trivialize this loop anyway. There is one integer field spanning all of the group separators and digits up to the decimal separator.
> 
> I could do a sort first which is O(NlogN) itself, but given the bounds of N (Number.MAX_NUMBER has ~100), I'm not sure it's worth the additional code complexity. The time here is dominated by unum_formatDoubleForFields.

Even if they are nested, you can sort an expected order by using beginIndex, and endIndex.

[    A   ]
[ B ]
     [ C ]

In this case, we can sort [A, B, C] if we use sort fields with beginIndex and then compare endIndex if beginIndex is the same.
Could you add FIXME, bug URL, and note about O(N^2) complexity?

>>> Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:635
>>> +        parts->push(&exec, part);
>> 
>> Do not use `push()` since it is used for ArrayPush and it is observable to users (it uses [[Put]]).
>> Use putDirectIndex instead.
> 
> Will do. `parts->putDirectIndex(&exec, parts->length(), part);` ?

Holding an index, and call `parts->putDirectIndex(&exec, index++, part);`.
Comment 31 Andy VanWagoner 2018-05-11 12:33:51 PDT
(In reply to Yusuke Suzuki from comment #30)
> Comment on attachment 340191 [details]
> Set test expectations properly
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=340191&action=review
> 
> >>> Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:628
> >>> +        }
> >> 
> >> I think if we sort this fields before iterating, we can iteratively accesses fields. In that case, time is O(NlogN), correct?
> > 
> > The fields are nested, so order doesn't trivialize this loop anyway. There is one integer field spanning all of the group separators and digits up to the decimal separator.
> > 
> > I could do a sort first which is O(NlogN) itself, but given the bounds of N (Number.MAX_NUMBER has ~100), I'm not sure it's worth the additional code complexity. The time here is dominated by unum_formatDoubleForFields.
> 
> Even if they are nested, you can sort an expected order by using beginIndex,
> and endIndex.
> 
> [    A   ]
> [ B ]
>      [ C ]
> 
> In this case, we can sort [A, B, C] if we use sort fields with beginIndex
> and then compare endIndex if beginIndex is the same.
> Could you add FIXME, bug URL, and note about O(N^2) complexity?

I spent some time trying to rework it to an O(N log N) algorithm, first sorting the fields, and then iterating through them, but I was having a rough time with it, so I'll add the FIXME and revisit another time: https://bugs.webkit.org/show_bug.cgi?id=185557

> 
> >>> Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:635
> >>> +        parts->push(&exec, part);
> >> 
> >> Do not use `push()` since it is used for ArrayPush and it is observable to users (it uses [[Put]]).
> >> Use putDirectIndex instead.
> > 
> > Will do. `parts->putDirectIndex(&exec, parts->length(), part);` ?
> 
> Holding an index, and call `parts->putDirectIndex(&exec, index++, part);`.

k, I'll fix that.
Comment 32 Andy VanWagoner 2018-05-11 13:29:16 PDT
Created attachment 340216 [details]
Patch
Comment 33 Andy VanWagoner 2018-05-14 10:25:38 PDT
Created attachment 340328 [details]
rebased
Comment 34 Andy VanWagoner 2018-05-14 10:26:37 PDT
Comment on attachment 340328 [details]
rebased

Fixed conflict and removed accidental change to license header.
Comment 35 Build Bot 2018-05-14 11:51:46 PDT
Comment on attachment 340328 [details]
rebased

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

New failing tests:
css3/filters/backdrop/add-remove-add-backdrop-filter.html
Comment 36 Build Bot 2018-05-14 11:51:48 PDT
Created attachment 340337 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 37 Andy VanWagoner 2018-05-15 10:20:50 PDT
Created attachment 340418 [details]
Patch
Comment 38 Andy VanWagoner 2018-05-16 12:30:33 PDT
Are there any other changes I should make, or is does this look ready to commit?
Comment 39 Yusuke Suzuki 2018-05-16 12:35:27 PDT
Comment on attachment 340418 [details]
Patch

r=me
Comment 40 WebKit Commit Bot 2018-05-16 12:37:55 PDT
Comment on attachment 340418 [details]
Patch

Rejecting attachment 340418 [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', 'apply-attachment', '--no-update', '--non-interactive', 340418, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
LayoutTests/platform/ios-simulator/TestExpectations
patching file LayoutTests/platform/mac/TestExpectations
Hunk #1 FAILED at 1762.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/mac/TestExpectations.rej
patching file LayoutTests/platform/win/TestExpectations
patching file ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Yusuke Suzuki']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.webkit.org/results/7700846
Comment 41 Yusuke Suzuki 2018-05-16 12:39:13 PDT
(In reply to WebKit Commit Bot from comment #40)
> Comment on attachment 340418 [details]
> Patch
> 
> Rejecting attachment 340418 [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',
> 'apply-attachment', '--no-update', '--non-interactive', 340418,
> '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit
> 
> Last 500 characters of output:
> LayoutTests/platform/ios-simulator/TestExpectations
> patching file LayoutTests/platform/mac/TestExpectations
> Hunk #1 FAILED at 1762.
> 1 out of 1 hunk FAILED -- saving rejects to file
> LayoutTests/platform/mac/TestExpectations.rej
> patching file LayoutTests/platform/win/TestExpectations
> patching file ChangeLog
> Hunk #1 succeeded at 1 with fuzz 3.
> 
> Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply',
> '--force', '--reviewer', u'Yusuke Suzuki']" exit_code: 1 cwd:
> /Volumes/Data/EWS/WebKit
> 
> Full output: http://webkit-queues.webkit.org/results/7700846

Could you rebase this?
Comment 42 Andy VanWagoner 2018-05-16 12:58:21 PDT
Created attachment 340516 [details]
rebased
Comment 43 WebKit Commit Bot 2018-05-16 13:38:39 PDT
Comment on attachment 340516 [details]
rebased

Clearing flags on attachment: 340516

Committed r231867: <https://trac.webkit.org/changeset/231867>
Comment 44 WebKit Commit Bot 2018-05-16 13:38:41 PDT
All reviewed patches have been landed.  Closing bug.
Comment 45 Radar WebKit Bug Importer 2018-05-16 13:42:01 PDT
<rdar://problem/40306553>
Comment 46 Robin Whittleton 2018-05-17 01:23:51 PDT
Thanks Andy for all the hard work, that was much quicker than I was expecting :)
Comment 47 Andy VanWagoner 2018-05-18 19:50:02 PDT
(In reply to Robin Whittleton from comment #46)
> Thanks Andy for all the hard work, that was much quicker than I was
> expecting :)

Happy to help. It's still not enabled on macOS, so it could still take a long while before it ends up in a Safari release.
Comment 48 Emanuele Feliziani 2019-03-26 00:51:27 PDT
This bug has been marked as RESOLVED FIXED for months, now, and the fix is already available in Safari Technology Preview. I was really hoping to see it included in Safari with macOS 10.14.4 and iOS 12.2, but it wasn't. Is there any ETA for this? What is holding this back?