Changeset 271735 in webkit


Ignore:
Timestamp:
Jan 21, 2021 7:50:35 PM (18 months ago)
Author:
Devin Rousso
Message:

[Apple Pay] use the first item in shippingOptions even when it's not selected
​https://bugs.webkit.org/show_bug.cgi?id=220810

Reviewed by Andy Estes.

LayoutTests/imported/w3c:

  • web-platform-tests/payment-request/payment-request-constructor.https.sub-expected.txt:
  • web-platform-tests/payment-request/payment-request-shippingOption-attribute.https-expected.txt:

Source/JavaScriptCore:

  • inspector/protocol/Console.json:
  • runtime/ConsoleTypes.h:
  • inspector/ConsoleMessage.cpp:

(Inspector::messageSourceValue):

  • runtime/ConsoleClient.cpp:

(JSC::appendMessagePrefix):
Add a new PaymentRequest value to the ChannelSource enum.

Source/WebCore:

The Payment Request API defines a selected property of PaymentShippingOption that is
used to control the initially selected shipping option when showing/updating a payment
request. Currently, PKShippingMethod does not have an equivalent concept and instead just
uses the first item in the -[PKPaymentRequest setShippingMethods:] such that there is
always a selected PKShippingMethod. This patch adjusts WebKit to follow those same
conventions, in that the first item in paymentDetailsBase.shippingOptions is used as the
selectedShippingOption so as to avoid situations where PassKit displays the first shipping
option while PaymentRequest.prototype.get shippingOption has a different value, which
could result in the user authorizing a payment with different shipping details than what
they were shown. Ideally in the future PassKit will add API/SPI to allow us to undo this.

Tests: http/tests/paymentrequest/payment-request-change-shipping-option.https.html

http/tests/paymentrequest/updateWith-shippingOptions.https.html
web-platform-tests/payment-request/payment-request-constructor.https.sub.html
web-platform-tests/payment-request/payment-request-shippingOption-attribute.https.html

  • Modules/paymentrequest/PaymentRequest.cpp:

(WebCore::checkAndCanonicalizeDetails):
(WebCore::PaymentRequest::create):
(WebCore::PaymentRequest::settleDetailsPromise):
Pass the ScriptExecutionContext instead of the JSGlobalObject so that it's possible to
call ScriptExecutionContext::addConsoleMessage. When creating a PaymentRequest, log if
subsequent PaymentShippingOption are marked as selected.

  • inspector/agents/page/PageConsoleAgent.cpp:

(WebCore::PageConsoleAgent::getLoggingChannels):
Add a new PaymentRequest value to the ChannelSource enum.

Source/WebInspectorUI:

  • UserInterface/Models/ConsoleMessage.js:
  • UserInterface/Models/IssueMessage.js:

(WI.IssueMessage):
Add a new PaymentRequest value to the ChannelSource enum.

Source/WebKitLegacy/mac:

  • WebCoreSupport/WebChromeClient.mm:

(stringForMessageSource):
Add a new PaymentRequest value to the ChannelSource enum.

LayoutTests:

  • http/tests/paymentrequest/payment-address-attributes-and-toJSON-method.https-expected.txt:
  • http/tests/paymentrequest/payment-request-change-shipping-option.https.html:
  • http/tests/paymentrequest/payment-request-change-shipping-option.https-expected.txt:
  • http/tests/paymentrequest/payment-response-retry-method.https-expected.txt:
  • http/tests/paymentrequest/updateWith-shippingOptions.https.html:
  • http/tests/paymentrequest/updateWith-shippingOptions.https-expected.txt:
Location:
trunk
Files:
23 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r271730 r271735  
     12021-01-21  Devin Rousso  <drousso@apple.com>
     2
     3        [Apple Pay] use the first item in `shippingOptions` even when it's not `selected`
     4        https://bugs.webkit.org/show_bug.cgi?id=220810
     5
     6        Reviewed by Andy Estes.
     7
     8        * http/tests/paymentrequest/payment-address-attributes-and-toJSON-method.https-expected.txt:
     9        * http/tests/paymentrequest/payment-request-change-shipping-option.https.html:
     10        * http/tests/paymentrequest/payment-request-change-shipping-option.https-expected.txt:
     11        * http/tests/paymentrequest/payment-response-retry-method.https-expected.txt:
     12        * http/tests/paymentrequest/updateWith-shippingOptions.https.html:
     13        * http/tests/paymentrequest/updateWith-shippingOptions.https-expected.txt:
     14
    1152021-01-21  Simon Fraser  <simon.fraser@apple.com>
    216

  • trunk/LayoutTests/http/tests/paymentrequest/payment-address-attributes-and-toJSON-method.https-expected.txt

    r267644 r271735  
     1CONSOLE MESSAGE: WebKit currently uses the first shipping option even if other shipping options are marked as selected.
     2CONSOLE MESSAGE: WebKit currently uses the first shipping option even if other shipping options are marked as selected.
    13If the requestShipping member is true, then shippingAddress's PaymentAddress must match the expected values.
    24

  • trunk/LayoutTests/http/tests/paymentrequest/payment-request-change-shipping-option.https-expected.txt

    r267644 r271735  
     1CONSOLE MESSAGE: WebKit currently uses the first shipping option even if other shipping options are marked as selected.
    12
    23PASS Test for PaymentRequest shippingOption attribute

  • trunk/LayoutTests/http/tests/paymentrequest/payment-request-change-shipping-option.https.html

    r226766 r271735  
    6464        internals.mockPaymentCoordinator.changeShippingOption("valid-1");
    6565    };
    66     assert_equals(
    67       request.shippingOption,
    68       "initially-selected",
    69       "Must be 'initially-selected', as the selected member is true"
    70     );
     66    // FIXME: <rdar://problem/73464404>
     67    // assert_equals(request.shippingOption, "initially-selected", "Must be 'initially-selected', as the selected member is true");
     68    assert_equals(request.shippingOption, "valid-1", "Must be the first shipping option regardless of the selected property.");
    7169    const listenerPromise = new Promise(resolve => {
    7270      request.addEventListener("shippingoptionchange", () => {

  • trunk/LayoutTests/http/tests/paymentrequest/payment-response-retry-method.https-expected.txt

    r267644 r271735  
     1CONSOLE MESSAGE: WebKit currently uses the first shipping option even if other shipping options are marked as selected.
     2CONSOLE MESSAGE: WebKit currently uses the first shipping option even if other shipping options are marked as selected.
     3CONSOLE MESSAGE: WebKit currently uses the first shipping option even if other shipping options are marked as selected.
     4CONSOLE MESSAGE: WebKit currently uses the first shipping option even if other shipping options are marked as selected.
     5CONSOLE MESSAGE: WebKit currently uses the first shipping option even if other shipping options are marked as selected.
     6CONSOLE MESSAGE: WebKit currently uses the first shipping option even if other shipping options are marked as selected.
     7CONSOLE MESSAGE: WebKit currently uses the first shipping option even if other shipping options are marked as selected.
     8CONSOLE MESSAGE: WebKit currently uses the first shipping option even if other shipping options are marked as selected.
    19
    210PASS Can construct a payment request (smoke test).

  • trunk/LayoutTests/http/tests/paymentrequest/updateWith-shippingOptions.https-expected.txt

    r271703 r271735  
     1CONSOLE MESSAGE: WebKit currently uses the first shipping option even if other shipping options are marked as selected.
    12
    23PASS Calling `updateWith` with a new `shippingOptions` without `requestShipping` should not update any values.

  • trunk/LayoutTests/http/tests/paymentrequest/updateWith-shippingOptions.https.html

    r271703 r271735  
    9999        assert_equals(internals.mockPaymentCoordinator.shippingMethods[i].amount, detailsUpdate.shippingOptions[i].amount.value, `shipping option ${i} amount should change`);
    100100    }
    101     assert_equals(request.shippingOption, detailsUpdate.shippingOptions.find((shippingOption) => shippingOption.selected).id, "selected shipping option should change");
     101    // FIXME: <rdar://problem/73464404>
     102    // assert_equals(request.shippingOption, detailsUpdate.shippingOptions.find((shippingOption) => shippingOption.selected).id, "selected shipping option should change");
     103    assert_equals(request.shippingOption, detailsUpdate.shippingOptions[0].id, "selected shipping option should change");
    102104    assert_equals(response.shippingOption, request.shippingOption, "selected shipping option should also be exposed on the response");
    103105

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r271734 r271735  
     12021-01-21  Devin Rousso  <drousso@apple.com>
     2
     3        [Apple Pay] use the first item in `shippingOptions` even when it's not `selected`
     4        https://bugs.webkit.org/show_bug.cgi?id=220810
     5
     6        Reviewed by Andy Estes.
     7
     8        * web-platform-tests/payment-request/payment-request-constructor.https.sub-expected.txt:
     9        * web-platform-tests/payment-request/payment-request-shippingOption-attribute.https-expected.txt:
     10
    1112021-01-21  Devin Rousso  <drousso@apple.com>
    212

  • trunk/LayoutTests/imported/w3c/web-platform-tests/payment-request/payment-request-constructor.https.sub-expected.txt

    r271734 r271735  
     1CONSOLE MESSAGE: WebKit currently uses the first shipping option even if other shipping options are marked as selected.
    12
    23PASS If details.id is missing, assign an identifier
    … …  
    1819PASS If there is no selected shipping option, then PaymentRequest.shippingOption remains null
    1920PASS If there is a selected shipping option, and requestShipping is set, then that option becomes synchronously selected
    20 PASS If requestShipping is set, and if there is a multiple selected shipping options, only the last is selected.
     21FAIL If requestShipping is set, and if there is a multiple selected shipping options, only the last is selected. assert_equals: selected option must 'the-id expected "the-id" but got "FAIL1"
    2122PASS If there are any duplicate shipping option ids, and shipping is requested, then throw a TypeError
    2223PASS Throw when there are duplicate shippingOption ids, even if other values are different

  • trunk/LayoutTests/imported/w3c/web-platform-tests/payment-request/payment-request-shippingOption-attribute.https-expected.txt

    r267647 r271735  
     1CONSOLE MESSAGE: WebKit currently uses the first shipping option even if other shipping options are marked as selected.
    12
    23PASS Must have a .shippingOption IDL attribute.
    34PASS .shippingOption attribute must default to null.
    4 PASS If there is a single shipping option, but selected is false, then .shippingOption must be null.
    5 PASS If there are multiple shipping options all with `selected` set to false, then .shippingOption is null.
     5FAIL If there is a single shipping option, but selected is false, then .shippingOption must be null. assert_equals: expected null expected (object) null but got (string) "valid"
     6FAIL If there are multiple shipping options all with `selected` set to false, then .shippingOption is null. assert_equals: expected null expected (object) null but got (string) "valid"
    67PASS Given multiple shipping options, it must use the selected shipping option for .shippingOption value.
    7 PASS If there are multiple of the shipping options with selected true, then .shippingOption is the last selected shipping option in order.
     8FAIL If there are multiple of the shipping options with selected true, then .shippingOption is the last selected shipping option in order. assert_equals: expected 'pass' expected "pass" but got "fail"
    89

  • trunk/Source/JavaScriptCore/ChangeLog

    r271731 r271735  
     12021-01-21  Devin Rousso  <drousso@apple.com>
     2
     3        [Apple Pay] use the first item in `shippingOptions` even when it's not `selected`
     4        https://bugs.webkit.org/show_bug.cgi?id=220810
     5
     6        Reviewed by Andy Estes.
     7
     8        * inspector/protocol/Console.json:
     9        * runtime/ConsoleTypes.h:
     10        * inspector/ConsoleMessage.cpp:
     11        (Inspector::messageSourceValue):
     12        * runtime/ConsoleClient.cpp:
     13        (JSC::appendMessagePrefix):
     14        Add a new `PaymentRequest` value to the `ChannelSource` enum.
     15
    1162021-01-21  Yusuke Suzuki  <ysuzuki@apple.com>
    217

  • trunk/Source/JavaScriptCore/inspector/ConsoleMessage.cpp

    r269712 r271735  
    193193    case MessageSource::ITPDebug: return Protocol::Console::ChannelSource::ITPDebug;
    194194    case MessageSource::PrivateClickMeasurement: return Protocol::Console::ChannelSource::PrivateClickMeasurement;
     195    case MessageSource::PaymentRequest: return Protocol::Console::ChannelSource::PaymentRequest;
    195196    case MessageSource::Other: return Protocol::Console::ChannelSource::Other;
    196197    }

  • trunk/Source/JavaScriptCore/inspector/protocol/Console.json

    r269712 r271735  
    2424                "itp-debug",
    2525                "private-click-measurement",
     26                "payment-request",
    2627                "other"
    2728            ],

  • trunk/Source/JavaScriptCore/runtime/ConsoleClient.cpp

    r269712 r271735  
    109109        sourceString = "PRIVATECLICKMEASUREMENT"_s;
    110110        break;
     111    case MessageSource::PaymentRequest:
     112        sourceString = "PAYMENTREQUEST"_s;
     113        break;
    111114    case MessageSource::Other:
    112115        sourceString = "OTHER"_s;

  • trunk/Source/JavaScriptCore/runtime/ConsoleTypes.h

    r269712 r271735  
    4646    ITPDebug,
    4747    PrivateClickMeasurement,
     48    PaymentRequest,
    4849    Other,
    4950};
    … …  
    9697        JSC::MessageSource::ITPDebug,
    9798        JSC::MessageSource::PrivateClickMeasurement,
     99        JSC::MessageSource::PaymentRequest,
    98100        JSC::MessageSource::Other
    99101    >;

  • trunk/Source/WebCore/ChangeLog

    r271734 r271735  
     12021-01-21  Devin Rousso  <drousso@apple.com>
     2
     3        [Apple Pay] use the first item in `shippingOptions` even when it's not `selected`
     4        https://bugs.webkit.org/show_bug.cgi?id=220810
     5
     6        Reviewed by Andy Estes.
     7
     8        The Payment Request API defines a `selected` property of `PaymentShippingOption` that is
     9        used to control the initially selected shipping option when showing/updating a payment
     10        request. Currently, `PKShippingMethod` does not have an equivalent concept and instead just
     11        uses the first item in the `-[PKPaymentRequest setShippingMethods:]` such that there is
     12        always a selected `PKShippingMethod`. This patch adjusts WebKit to follow those same
     13        conventions, in that the first item in `paymentDetailsBase.shippingOptions` is used as the
     14        `selectedShippingOption` so as to avoid situations where PassKit displays the first shipping
     15        option while `PaymentRequest.prototype.get shippingOption` has a different value, which
     16        could result in the user authorizing a payment with different shipping details than what
     17        they were shown. Ideally in the future PassKit will add API/SPI to allow us to undo this.
     18
     19        Tests: http/tests/paymentrequest/payment-request-change-shipping-option.https.html
     20               http/tests/paymentrequest/updateWith-shippingOptions.https.html
     21               web-platform-tests/payment-request/payment-request-constructor.https.sub.html
     22               web-platform-tests/payment-request/payment-request-shippingOption-attribute.https.html
     23
     24        * Modules/paymentrequest/PaymentRequest.cpp:
     25        (WebCore::checkAndCanonicalizeDetails):
     26        (WebCore::PaymentRequest::create):
     27        (WebCore::PaymentRequest::settleDetailsPromise):
     28        Pass the `ScriptExecutionContext` instead of the `JSGlobalObject` so that it's possible to
     29        call `ScriptExecutionContext::addConsoleMessage`. When creating a `PaymentRequest`, log if
     30        subsequent `PaymentShippingOption` are marked as `selected`.
     31
     32        * inspector/agents/page/PageConsoleAgent.cpp:
     33        (WebCore::PageConsoleAgent::getLoggingChannels):
     34        Add a new `PaymentRequest` value to the `ChannelSource` enum.
     35
    1362021-01-21  Devin Rousso  <drousso@apple.com>
    237

  • trunk/Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp

    r271734 r271735  
    246246};
    247247
    248 static ExceptionOr<std::tuple<String, Vector<String>>> checkAndCanonicalizeDetails(JSC::JSGlobalObject& execState, PaymentDetailsBase& details, bool requestShipping, IsUpdate isUpdate)
     248static ExceptionOr<std::tuple<String, Vector<String>>> checkAndCanonicalizeDetails(ScriptExecutionContext& context, PaymentDetailsBase& details, bool requestShipping, IsUpdate isUpdate)
    249249{
    250250    if (details.displayItems) {
    … …  
    260260        if (details.shippingOptions) {
    261261            HashSet<String> seenShippingOptionIDs;
     262            bool didLog = false;
    262263            for (auto& shippingOption : *details.shippingOptions) {
    263264                auto exception = checkAndCanonicalizeAmount(shippingOption.amount);
    … …  
    269270                    return Exception { TypeError, "Shipping option IDs must be unique." };
    270271
    271                 if (shippingOption.selected)
     272                // FIXME: <rdar://problem/73464404>
     273                if (!selectedShippingOption)
    272274                    selectedShippingOption = shippingOption.id;
     275                else if (!didLog && shippingOption.selected) {
     276                    context.addConsoleMessage(JSC::MessageSource::PaymentRequest, JSC::MessageLevel::Warning, "WebKit currently uses the first shipping option even if other shipping options are marked as selected."_s);
     277                    didLog = true;
     278                }
    273279            }
    274280        } else if (isUpdate == IsUpdate::No)
    … …  
    300306            String serializedData;
    301307            if (modifier.data) {
    302                 auto scope = DECLARE_THROW_SCOPE(execState.vm());
    303                 serializedData = JSONStringify(&execState, modifier.data.get(), 0);
     308                auto* globalObject = context.globalObject();
     309                auto scope = DECLARE_THROW_SCOPE(globalObject->vm());
     310                serializedData = JSONStringify(globalObject, modifier.data.get(), 0);
    304311                if (scope.exception())
    305312                    return Exception { ExistingExceptionError };
    … …  
    378385        return totalResult.releaseException();
    379386
    380     auto detailsResult = checkAndCanonicalizeDetails(*document.globalObject(), details, options.requestShipping, IsUpdate::No);
     387    auto detailsResult = checkAndCanonicalizeDetails(document, details, options.requestShipping, IsUpdate::No);
    381388    if (detailsResult.hasException())
    382389        return detailsResult.releaseException();
    … …  
    682689    }
    683690
    684     auto detailsResult = checkAndCanonicalizeDetails(*context.globalObject(), detailsUpdate, m_options.requestShipping, IsUpdate::Yes);
     691    auto detailsResult = checkAndCanonicalizeDetails(context, detailsUpdate, m_options.requestShipping, IsUpdate::Yes);
    685692    if (detailsResult.hasException()) {
    686693        abortWithException(detailsResult.releaseException());

  • trunk/Source/WebCore/inspector/agents/page/PageConsoleAgent.cpp

    r269712 r271735  
    109109    addLogChannel(Protocol::Console::ChannelSource::ITPDebug);
    110110    addLogChannel(Protocol::Console::ChannelSource::PrivateClickMeasurement);
     111    addLogChannel(Protocol::Console::ChannelSource::PaymentRequest);
    111112    addLogChannel(Protocol::Console::ChannelSource::Other);
    112113

  • trunk/Source/WebInspectorUI/ChangeLog

    r271726 r271735  
     12021-01-21  Devin Rousso  <drousso@apple.com>
     2
     3        [Apple Pay] use the first item in `shippingOptions` even when it's not `selected`
     4        https://bugs.webkit.org/show_bug.cgi?id=220810
     5
     6        Reviewed by Andy Estes.
     7
     8        * UserInterface/Models/ConsoleMessage.js:
     9        * UserInterface/Models/IssueMessage.js:
     10        (WI.IssueMessage):
     11        Add a new `PaymentRequest` value to the `ChannelSource` enum.
     12
    1132021-01-21  Ebrahim Byagowi  <ebrahim@gnu.org>
    214

  • trunk/Source/WebInspectorUI/UserInterface/Models/ConsoleMessage.js

    r269712 r271735  
    115115    ITPDebug: "itp-debug",
    116116    PrivateClickMeasurement: "private-click-measurement",
     117    PaymentRequest: "payment-request",
    117118    Other: "other",
    118119

  • trunk/Source/WebInspectorUI/UserInterface/Models/IssueMessage.js

    r269712 r271735  
    7171        case WI.ConsoleMessage.MessageSource.ITPDebug:
    7272        case WI.ConsoleMessage.MessageSource.PrivateClickMeasurement:
     73        case WI.ConsoleMessage.MessageSource.PaymentRequest:
    7374        case WI.ConsoleMessage.MessageSource.AdClickAttribution: // COMPATIBILITY (iOS 14.0): `Console.ChannelSource.AdClickAttribution` was renamed to `Console.ChannelSource.PrivateClickMeasurement`.
    7475        case WI.ConsoleMessage.MessageSource.Other:

  • trunk/Source/WebKitLegacy/mac/ChangeLog

    r271685 r271735  
     12021-01-21  Devin Rousso  <drousso@apple.com>
     2
     3        [Apple Pay] use the first item in `shippingOptions` even when it's not `selected`
     4        https://bugs.webkit.org/show_bug.cgi?id=220810
     5
     6        Reviewed by Andy Estes.
     7
     8        * WebCoreSupport/WebChromeClient.mm:
     9        (stringForMessageSource):
     10        Add a new `PaymentRequest` value to the `ChannelSource` enum.
     11
    1122021-01-20  Wenson Hsieh  <wenson_hsieh@apple.com>
    213

  • trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.mm

    r270577 r271735  
    127127NSString *WebConsoleMessageITPDebugMessageSource = @"ITPDebugMessageSource";
    128128NSString *WebConsoleMessagePrivateClickMeasurementMessageSource = @"PrivateClickMeasurementMessageSource";
     129NSString *WebConsoleMessagePaymentRequestMessageSource = @"PaymentRequestMessageSource";
    129130NSString *WebConsoleMessageOtherMessageSource = @"OtherMessageSource";
    130131
    … …  
    407408    case MessageSource::PrivateClickMeasurement:
    408409        return WebConsoleMessagePrivateClickMeasurementMessageSource;
     410    case MessageSource::PaymentRequest:
     411        return WebConsoleMessagePaymentRequestMessageSource;
    409412    case MessageSource::Other:
    410413        return WebConsoleMessageOtherMessageSource;
Note: See TracChangeset for help on using the changeset viewer.