Summary: | Default attachment icon is not necessarily the same as the file attachment icon whose extension is ".dat" | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Said Abou-Hallawa <sabouhallawa> | ||||||
Component: | New Bugs | Assignee: | Said Abou-Hallawa <sabouhallawa> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, rniwa, thorton, webkit-bug-importer, wenson_hsieh | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=176769 | ||||||||
Attachments: |
|
Description
Said Abou-Hallawa
2020-03-17 19:27:15 PDT
Created attachment 393813 [details]
Patch
Comment on attachment 393813 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393813&action=review This patch looks good. > LayoutTests/ChangeLog:9 > + returns for Ć¢public.dataĆ¢ icon. Ok as-is. No change needed. There are some fancy quotes here. Comment on attachment 393813 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393813&action=review > LayoutTests/fast/attachment/attachment-default-icon-expected.html:4 > +<attachment id="attachment" title=" " type="public.data"></attachment> After looking at other attachment tests, I think I understand what this test was going for....it's trying to test the code path for the file attribute matches the code path for the missing type attribute (which yields the default icon). The implicit assumption this test makes is that a .dat file yields the default icon. For this test (and only this test as I haven't studied the others enough,yet) this patch seems reasonable because: 1. There are other tests that actually check that the file attribute affects the icon 2. Because of (1) this test just needs to focus on testing that the icon is for the missing type attribute is equivalent to the icon for a .dat file. Comment on attachment 393813 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393813&action=review > LayoutTests/ChangeLog:5 > + Ok as-is. No change needed. The optimal ChangeLog includes the radar URL here because: 1. Convenient for Apple engineers Created attachment 393854 [details]
Patch
The commit-queue encountered the following flaky tests while processing attachment 393854 [details]: imported/w3c/web-platform-tests/IndexedDB/interleaved-cursors-large.html bug 201849 The commit-queue is continuing to process your patch. Comment on attachment 393854 [details] Patch Clearing flags on attachment: 393854 Committed r258640: <https://trac.webkit.org/changeset/258640> All reviewed patches have been landed. Closing bug. |