[Payment Request] Begin implementing PaymentMethodChangeEvent
Created attachment 348438 [details] Patch
Created attachment 348439 [details] Patch
Attachment 348439 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.cpp:42: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 20 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 348439 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=348439&action=review > LayoutTests/imported/w3c/ChangeLog:11 > + One test in onpaymentmenthodchange-attribute.https.html continues to fail due to a bug in > + the test. Oops! This is a bug in the patch, not the test. Fixing.
Created attachment 348463 [details] Patch
Attachment 348463 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.cpp:42: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 348479 [details] Patch
Attachment 348479 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.cpp:42: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 348520 [details] Patch
Attachment 348520 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.cpp:42: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 348520 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=348520&action=review r=me > Source/WebCore/ChangeLog:9 > + by <https://w3c.github.io/payment-request/#paymentmethodchangeevent-interface>. For historical preservation, please include the date of the draft you referenced. > Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.cpp:27 > +#include "PaymentMethodChangeEvent.h" I suggest we move this under the ENABLE(PAYMENT_REQUEST)-guard. > Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.cpp:40 > +PaymentMethodChangeEvent::PaymentMethodChangeEvent(const AtomicString& type, const PaymentMethodChangeEventInit& eventInit) Can we make the first parameter an rvalue reference and move it into PaymentRequestUpdateEvent? > Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.h:40 > +struct PaymentMethodChangeEventInit; This forward declaration is unnecessary as PaymentRequestUpdateEvent.h already contains it. > Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.h:44 > + template <typename... Args> static Ref<PaymentMethodChangeEvent> create(Args&&... args) Nit: We do not put a space between "template" and the template parameters. The code base has a mix of both kinds. The clear majority is without the space. > Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEventInit.h:39 > +struct PaymentMethodChangeEventInit final : public PaymentRequestUpdateEventInit { The keyword "public" is not necessary. > Source/WebCore/Modules/paymentrequest/PaymentRequestUpdateEventInit.h:34 > +struct PaymentRequestUpdateEventInit : public EventInit { Ditto.
(In reply to Daniel Bates from comment #11) > Comment on attachment 348520 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=348520&action=review > > r=me > > > Source/WebCore/ChangeLog:9 > > + by <https://w3c.github.io/payment-request/#paymentmethodchangeevent-interface>. > > For historical preservation, please include the date of the draft you > referenced. Ok. > > > Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.cpp:27 > > +#include "PaymentMethodChangeEvent.h" > > I suggest we move this under the ENABLE(PAYMENT_REQUEST)-guard. Ok. > > > Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.cpp:40 > > +PaymentMethodChangeEvent::PaymentMethodChangeEvent(const AtomicString& type, const PaymentMethodChangeEventInit& eventInit) > > Can we make the first parameter an rvalue reference and move it into > PaymentRequestUpdateEvent? I don't think so, because then I'd initialize m_methodName and m_methodDetails using a moved-from value. > > > Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.h:40 > > +struct PaymentMethodChangeEventInit; > > This forward declaration is unnecessary as PaymentRequestUpdateEvent.h > already contains it. PaymentRequestUpdateEvent.h contains a forward declaration of PaymentRequestUpdateEventInit, not PaymentMethodChangeEventInit. > > > Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.h:44 > > + template <typename... Args> static Ref<PaymentMethodChangeEvent> create(Args&&... args) > > Nit: We do not put a space between "template" and the template parameters. > The code base has a mix of both kinds. The clear majority is without the > space. > > > Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEventInit.h:39 > > +struct PaymentMethodChangeEventInit final : public PaymentRequestUpdateEventInit { > > The keyword "public" is not necessary. > > > Source/WebCore/Modules/paymentrequest/PaymentRequestUpdateEventInit.h:34 > > +struct PaymentRequestUpdateEventInit : public EventInit { > > Ditto. Fixed.
Created attachment 348527 [details] Patch
Attachment 348527 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.cpp:42: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 348527 [details] Patch Clearing flags on attachment: 348527 Committed r235518: <https://trac.webkit.org/changeset/235518>
All reviewed patches have been landed. Closing bug.
<rdar://problem/43941938>