Bug 232649

Summary: Notifications on iOS enabled at compile-time, disabled at runtime
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebKit Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, calvaris, cdumez, cmarcelo, esprehn+autocc, ews-watchlist, kondapallykalyan, thorton, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
[fast-cq] Disable these API tests on iOS
beidson: commit-queue-
[fast-cq] Disable these API tests on iOS none

Description Brady Eidson 2021-11-02 18:04:09 PDT
Notifications on iOS enabled at compile-time, disabled at runtime
Comment 1 Brady Eidson 2021-11-02 18:06:09 PDT
Created attachment 443154 [details]
Patch
Comment 2 Tim Horton 2021-11-02 18:08:21 PDT
Comment on attachment 443154 [details]
Patch

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

> Source/WebCore/bindings/js/WebCoreBuiltinNames.h:186
> +    macro(Notification) \

plural?
Comment 3 Brady Eidson 2021-11-02 18:16:51 PDT
(In reply to Tim Horton from comment #2)
> Comment on attachment 443154 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=443154&action=review
> 
> > Source/WebCore/bindings/js/WebCoreBuiltinNames.h:186
> > +    macro(Notification) \
> 
> plural?

The IDL object being controlled by the setting here is, indeed, "Notification" singular.
Comment 4 Tim Horton 2021-11-02 18:34:27 PDT
Comment on attachment 443154 [details]
Patch

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

>>> Source/WebCore/bindings/js/WebCoreBuiltinNames.h:186
>>> +    macro(Notification) \
>> 
>> plural?
> 
> The IDL object being controlled by the setting here is, indeed, "Notification" singular.

Oh I totally missed what this was (A+ review), thought it was Logging.h :P
Comment 5 Brady Eidson 2021-11-02 18:49:43 PDT
(In reply to Tim Horton from comment #4)
> Comment on attachment 443154 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=443154&action=review
> 
> >>> Source/WebCore/bindings/js/WebCoreBuiltinNames.h:186
> >>> +    macro(Notification) \
> >> 
> >> plural?
> > 
> > The IDL object being controlled by the setting here is, indeed, "Notification" singular.
> 
> Oh I totally missed what this was (A+ review), thought it was Logging.h :P

A++++++ would have review again!
Comment 6 EWS 2021-11-02 23:53:20 PDT
Committed r285199 (243825@main): <https://commits.webkit.org/243825@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 443154 [details].
Comment 7 Radar WebKit Bug Importer 2021-11-02 23:54:18 PDT
<rdar://problem/84963287>
Comment 8 Brady Eidson 2021-11-03 09:02:20 PDT
Yay for being bitten by no API test EWS again. *sigh*

Fixing.
Comment 9 Brady Eidson 2021-11-03 09:09:56 PDT
Created attachment 443201 [details]
[fast-cq] Disable these API tests on iOS
Comment 10 Brady Eidson 2021-11-03 09:10:27 PDT
Reopened for API test failure.
Comment 11 EWS 2021-11-03 09:11:59 PDT
ChangeLog entry in Tools/ChangeLog contains OOPS!.
Comment 12 Brady Eidson 2021-11-03 09:12:20 PDT
Created attachment 443202 [details]
[fast-cq] Disable these API tests on iOS
Comment 13 EWS 2021-11-03 09:14:14 PDT
Committed r285207 (243833@main): <https://commits.webkit.org/243833@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 443202 [details].