233764 2021-12-02 09:54:24 -0800 Make enum values consistent in ApplicationManifest::Icon::Purpose and _WKApplicationManifestIconPurpose 2021-12-02 17:42:24 -0800 1 1 1 Unclassified WebKit WebCore Misc. Safari 15 Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 rginsberg webkit-unassigned bfulgham rginsberg webkit-bug-importer oldest_to_newest 1819739 0 rginsberg 2021-12-02 09:54:24 -0800 `Icon::Purpose` values are 1 (`1 << 0`), 2 (`1 << 1`), and 4 (`1 << 2`), but the default values for an enum are 0, 1, 2, which is used in `_WKApplicationManifestIcon`. This results in the purpose values being mis-translated in `_WKApplicationManifest`. In `WKApplicationManifest` an icon with the `purpose` set to `Any` in the manifest file, will have it's `purposes` set to `_WKApplicationManifestIconPurposeMonochrome`. 1819740 1 webkit-bug-importer 2021-12-02 09:54:45 -0800 <rdar://problem/85976270> 1819788 2 445744 rginsberg 2021-12-02 10:56:22 -0800 Created attachment 445744 Patch 1819859 3 445744 bfulgham 2021-12-02 14:03:54 -0800 Comment on attachment 445744 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445744&action=review This looks fine, but I suggest following the model of the underling WebCore type and use the bitwise-shift to show the mask values. > Source/WebKit/UIProcess/API/Cocoa/_WKApplicationManifest.h:46 > + _WKApplicationManifestIconPurposeAny = 1, We should follow the behavior of the WebCore::ApplicationManifest::Icon::Purpose enum: _WKApplicationManifestIconPurposeAny = (1 << 0), > Source/WebKit/UIProcess/API/Cocoa/_WKApplicationManifest.h:47 > + _WKApplicationManifestIconPurposeMonochrome = 2, _WKApplicationManifestIconPurposeMonochrome = (1 << 1), > Source/WebKit/UIProcess/API/Cocoa/_WKApplicationManifest.h:48 > + _WKApplicationManifestIconPurposeMaskable = 4, _WKApplicationManifestIconPurposeMaskable = (1 << 2), 1819889 4 445778 rginsberg 2021-12-02 15:11:58 -0800 Created attachment 445778 Patch 1819890 5 rginsberg 2021-12-02 15:12:23 -0800 (In reply to Brent Fulgham from comment #3) > Comment on attachment 445744 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=445744&action=review > > This looks fine, but I suggest following the model of the underling WebCore > type and use the bitwise-shift to show the mask values. > > > Source/WebKit/UIProcess/API/Cocoa/_WKApplicationManifest.h:46 > > + _WKApplicationManifestIconPurposeAny = 1, > > We should follow the behavior of the > WebCore::ApplicationManifest::Icon::Purpose enum: > > _WKApplicationManifestIconPurposeAny = (1 << 0), > > > Source/WebKit/UIProcess/API/Cocoa/_WKApplicationManifest.h:47 > > + _WKApplicationManifestIconPurposeMonochrome = 2, > > _WKApplicationManifestIconPurposeMonochrome = (1 << 1), > > > Source/WebKit/UIProcess/API/Cocoa/_WKApplicationManifest.h:48 > > + _WKApplicationManifestIconPurposeMaskable = 4, > > _WKApplicationManifestIconPurposeMaskable = (1 << 2), Thanks! Just uploaded a patch that uses this model 1819893 6 445778 bfulgham 2021-12-02 15:16:26 -0800 Comment on attachment 445778 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445778&action=review r=me > Source/WebKit/UIProcess/API/Cocoa/_WKApplicationManifest.h:48 > + _WKApplicationManifestIconPurposeMaskable = (1 << 2), Thank you for adjusting this! 1819894 7 bfulgham 2021-12-02 15:16:49 -0800 I'll cq+ this change once EWS finishes. 1819975 8 ews-feeder 2021-12-02 17:42:23 -0800 Committed r286470 (244810@main): <https://commits.webkit.org/244810@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 445778. 445744 2021-12-02 10:56:22 -0800 2021-12-02 15:11:58 -0800 Patch file_233764.txt text/plain 2065 rginsberg Y29tbWl0IDJmMmMwZmRlODBkY2UyYTMwZGRkMzdkMDEyM2E1MjAzNzJkMTgwZmQNCkF1dGhvcjog UmFjaGVsIEdpbnNiZXJnIDxyZ2luc2JlcmdAYXBwbGUuY29tPg0KRGF0ZTogICBUaHUgRGVjIDIg MTM6NTE6MjEgMjAyMSAtMDUwMA0KDQogICAgZmlyc3QgY29tbWl0DQoNCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViS2l0L0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nDQppbmRleCA1 MzczMjZiYTA4NWUuLmNhNmRmNmQ4ODZiZSAxMDA2NDQNCi0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hh bmdlTG9nDQorKysgYi9Tb3VyY2UvV2ViS2l0L0NoYW5nZUxvZw0KQEAgLTEsMyArMSwyMSBAQA0K KzIwMjEtMTItMDIgIFJhY2hlbCBHaW5zYmVyZyAgPHJnaW5zYmVyZ0BhcHBsZS5jb20+DQorDQor ICAgICAgICBNYWtlIGVudW0gdmFsdWVzIGNvbnNpc3RlbiBpbiBBcHBsaWNhdGlvbk1hbmlmZXN0 OjpJY29uOjpQdXJwb3NlDQorICAgICAgICBhbmQgX1dLQXBwbGljYXRpb25NYW5pZmVzdEljb25Q dXJwb3NlDQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MjMzNzY0DQorICAgICAgICByZGFyOi8vODU5NzYyNzANCisNCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLg0KKw0KKyAgICAgICAgVGhlIEFwcGxpY2F0aW9uTWFuaWZlc3QgdGVz dHMgYWxyZWFkeSBjaGVjayBmb3IgMSwgMiwgYW5kIDQgcmVzcGVjdGl2ZWx5Lg0KKw0KKyAgICAg ICAgKiBVSVByb2Nlc3MvQVBJL0NvY29hL19XS0FwcGxpY2F0aW9uTWFuaWZlc3QuaDoNCisgICAg ICAgIFNldCB0aGUgX1dLQXBwbGljYXRpb25NYW5pZmVzdEljb25QdXJwb3NlIGVudW0gdmFsdWVz IHRvIA0KKyAgICAgICAgMSwgMiwgNCB0byBtYXRjaCB0aGUgZW51bSB2YWx1ZXMgc2V0IGluIEFw cGxpY2F0aW9uTWFuaWZlc3Q6Okljb246OlB1cnBvc2UuDQorICAgICAgICBUaG9zZSB2YWx1ZXMg YXJlIHVzZWQgaW4gQXBwbGljYXRpb25NYW5pZmVzdCBiZWNhdXNlIGFuDQorICAgICAgICBPcHRp b25TZXQgaXMgdXNlZC4NCisNCisNCiAyMDIxLTEyLTAyICBZb3Vlbm4gRmFibGV0ICA8eW91ZW5u QGFwcGxlLmNvbT4NCiANCiAgICAgICAgIEFkZCBzb21lIGxvZ2dpbmcgdG8gTmV0d29ya1JUQ1By b3ZpZGVyDQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvQVBJL0NvY29hL19X S0FwcGxpY2F0aW9uTWFuaWZlc3QuaCBiL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL0FQSS9Db2Nv YS9fV0tBcHBsaWNhdGlvbk1hbmlmZXN0LmgNCmluZGV4IDA4ZDI5OWZjYjdjOC4uMWRkZjRjYWE4 YjZhIDEwMDY0NA0KLS0tIGEvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvQVBJL0NvY29hL19XS0Fw cGxpY2F0aW9uTWFuaWZlc3QuaA0KKysrIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvQVBJL0Nv Y29hL19XS0FwcGxpY2F0aW9uTWFuaWZlc3QuaA0KQEAgLTQzLDkgKzQzLDkgQEAgdHlwZWRlZiBO U19FTlVNKE5TSW50ZWdlciwgX1dLQXBwbGljYXRpb25NYW5pZmVzdERpc3BsYXlNb2RlKSB7DQog fSBXS19BUElfQVZBSUxBQkxFKG1hY29zKDEwLjEzLjQpLCBpb3MoMTEuMykpOw0KIA0KIHR5cGVk ZWYgTlNfRU5VTShOU0ludGVnZXIsIF9XS0FwcGxpY2F0aW9uTWFuaWZlc3RJY29uUHVycG9zZSkg ew0KLSAgICBfV0tBcHBsaWNhdGlvbk1hbmlmZXN0SWNvblB1cnBvc2VBbnksDQotICAgIF9XS0Fw cGxpY2F0aW9uTWFuaWZlc3RJY29uUHVycG9zZU1vbm9jaHJvbWUsDQotICAgIF9XS0FwcGxpY2F0 aW9uTWFuaWZlc3RJY29uUHVycG9zZU1hc2thYmxlLA0KKyAgICBfV0tBcHBsaWNhdGlvbk1hbmlm ZXN0SWNvblB1cnBvc2VBbnkgPSAxLA0KKyAgICBfV0tBcHBsaWNhdGlvbk1hbmlmZXN0SWNvblB1 cnBvc2VNb25vY2hyb21lID0gMiwNCisgICAgX1dLQXBwbGljYXRpb25NYW5pZmVzdEljb25QdXJw b3NlTWFza2FibGUgPSA0LA0KIH0gV0tfQVBJX0FWQUlMQUJMRShtYWNvcyhXS19NQUNfVEJBKSwg aW9zKFdLX0lPU19UQkEpKTsNCiANCiBXS19DTEFTU19BVkFJTEFCTEUobWFjb3MoMTAuMTMuNCks IGlvcygxMS4zKSkNCg== 445778 2021-12-02 15:11:58 -0800 2021-12-02 17:42:23 -0800 Patch file_233764.txt text/plain 1929 rginsberg ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nIGIvU291cmNlL1dlYktpdC9DaGFu Z2VMb2cNCmluZGV4IDUzNzMyNmJhMDg1ZS4uY2E2ZGY2ZDg4NmJlIDEwMDY0NA0KLS0tIGEvU291 cmNlL1dlYktpdC9DaGFuZ2VMb2cNCisrKyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nDQpAQCAt MSwzICsxLDIxIEBADQorMjAyMS0xMi0wMiAgUmFjaGVsIEdpbnNiZXJnICA8cmdpbnNiZXJnQGFw cGxlLmNvbT4NCisNCisgICAgICAgIE1ha2UgZW51bSB2YWx1ZXMgY29uc2lzdGVuIGluIEFwcGxp Y2F0aW9uTWFuaWZlc3Q6Okljb246OlB1cnBvc2UNCisgICAgICAgIGFuZCBfV0tBcHBsaWNhdGlv bk1hbmlmZXN0SWNvblB1cnBvc2UNCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD0yMzM3NjQNCisgICAgICAgIHJkYXI6Ly84NTk3NjI3MA0KKw0KKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuDQorDQorICAgICAgICBUaGUgQXBwbGljYXRp b25NYW5pZmVzdCB0ZXN0cyBhbHJlYWR5IGNoZWNrIGZvciAxLCAyLCBhbmQgNCByZXNwZWN0aXZl bHkuDQorDQorICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvQ29jb2EvX1dLQXBwbGljYXRpb25NYW5p ZmVzdC5oOg0KKyAgICAgICAgU2V0IHRoZSBfV0tBcHBsaWNhdGlvbk1hbmlmZXN0SWNvblB1cnBv c2UgZW51bSB2YWx1ZXMgdG8gDQorICAgICAgICAxLCAyLCA0IHRvIG1hdGNoIHRoZSBlbnVtIHZh bHVlcyBzZXQgaW4gQXBwbGljYXRpb25NYW5pZmVzdDo6SWNvbjo6UHVycG9zZS4NCisgICAgICAg IFRob3NlIHZhbHVlcyBhcmUgdXNlZCBpbiBBcHBsaWNhdGlvbk1hbmlmZXN0IGJlY2F1c2UgYW4N CisgICAgICAgIE9wdGlvblNldCBpcyB1c2VkLg0KKw0KKw0KIDIwMjEtMTItMDIgIFlvdWVubiBG YWJsZXQgIDx5b3Vlbm5AYXBwbGUuY29tPg0KIA0KICAgICAgICAgQWRkIHNvbWUgbG9nZ2luZyB0 byBOZXR3b3JrUlRDUHJvdmlkZXINCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vz cy9BUEkvQ29jb2EvX1dLQXBwbGljYXRpb25NYW5pZmVzdC5oIGIvU291cmNlL1dlYktpdC9VSVBy b2Nlc3MvQVBJL0NvY29hL19XS0FwcGxpY2F0aW9uTWFuaWZlc3QuaA0KaW5kZXggMDhkMjk5ZmNi N2M4Li5hZTEwYmMyMGQwYmIgMTAwNjQ0DQotLS0gYS9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9B UEkvQ29jb2EvX1dLQXBwbGljYXRpb25NYW5pZmVzdC5oDQorKysgYi9Tb3VyY2UvV2ViS2l0L1VJ UHJvY2Vzcy9BUEkvQ29jb2EvX1dLQXBwbGljYXRpb25NYW5pZmVzdC5oDQpAQCAtNDMsOSArNDMs OSBAQCB0eXBlZGVmIE5TX0VOVU0oTlNJbnRlZ2VyLCBfV0tBcHBsaWNhdGlvbk1hbmlmZXN0RGlz cGxheU1vZGUpIHsNCiB9IFdLX0FQSV9BVkFJTEFCTEUobWFjb3MoMTAuMTMuNCksIGlvcygxMS4z KSk7DQogDQogdHlwZWRlZiBOU19FTlVNKE5TSW50ZWdlciwgX1dLQXBwbGljYXRpb25NYW5pZmVz dEljb25QdXJwb3NlKSB7DQotICAgIF9XS0FwcGxpY2F0aW9uTWFuaWZlc3RJY29uUHVycG9zZUFu eSwNCi0gICAgX1dLQXBwbGljYXRpb25NYW5pZmVzdEljb25QdXJwb3NlTW9ub2Nocm9tZSwNCi0g ICAgX1dLQXBwbGljYXRpb25NYW5pZmVzdEljb25QdXJwb3NlTWFza2FibGUsDQorICAgIF9XS0Fw cGxpY2F0aW9uTWFuaWZlc3RJY29uUHVycG9zZUFueSA9ICgxIDw8IDApLA0KKyAgICBfV0tBcHBs aWNhdGlvbk1hbmlmZXN0SWNvblB1cnBvc2VNb25vY2hyb21lID0gKDEgPDwgMSksDQorICAgIF9X S0FwcGxpY2F0aW9uTWFuaWZlc3RJY29uUHVycG9zZU1hc2thYmxlID0gKDEgPDwgMiksDQogfSBX S19BUElfQVZBSUxBQkxFKG1hY29zKFdLX01BQ19UQkEpLCBpb3MoV0tfSU9TX1RCQSkpOw0KIA0K IFdLX0NMQVNTX0FWQUlMQUJMRShtYWNvcygxMC4xMy40KSwgaW9zKDExLjMpKQ0K