| Summary: | Streamline icon-related code, mostly unused | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Darin Adler <darin> | ||||||||||||
| Component: | Page Loading | Assignee: | Darin Adler <darin> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | andersca, beidson, buildbot, gyuyoung.kim, kling, koivisto, rniwa, sam | ||||||||||||
| Priority: | P2 | ||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Darin Adler
2015-04-01 08:42:03 PDT
Created attachment 249980 [details]
Patch
Created attachment 249986 [details]
Patch
Comment on attachment 249986 [details] Patch Attachment 249986 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6456446019436544 New failing tests: fast/dom/icon-url-list.html fast/dom/icon-url-list-apple-touch.html Created attachment 249990 [details]
Archive of layout-test-results from ews102 for mac-mavericks
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-mavericks Platform: Mac OS X 10.9.5
Comment on attachment 249986 [details] Patch Attachment 249986 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5134296171937792 New failing tests: fast/dom/icon-url-list.html fast/dom/icon-url-list-apple-touch.html Created attachment 249996 [details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 249986 [details]
Patch
Oops, looks like I uploaded a partial patch *without* the changes to LayoutTests. That’s why tests are failing.
Created attachment 250031 [details]
Patch
Ready to review now. All the tests are passing now. Comment on attachment 250031 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250031&action=review > Source/WebCore/ChangeLog:20 > + time, with some small exceptoins, icon logic is in the web browsers, not in WebKit. typo "exceptoins" > Source/WebCore/dom/IconURL.h:38 > enum IconType { enum class would be nice > Source/WebCore/html/LinkRelAttribute.h:41 > + bool isStyleSheet { false }; I have been wondering if we should use { false } or { }. So far I have stayed with the former though it is redundant. Comment on attachment 250031 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250031&action=review > Source/WebCore/loader/icon/IconController.cpp:55 > +enum LinkElementSelector { AllLinkElements, LinkElementsWithMIMETypes }; enum class SelectLinkElements { All, WithMIMEType }? Comment on attachment 250031 [details]
Patch
OK. I’ll make some changes tonight before I land this.
Comment on attachment 250031 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250031&action=review >> Source/WebCore/dom/IconURL.h:38 >> enum IconType { > > enum class would be nice This was a little inconvenient to do quickly. I’m going to wait on that for a later patch. Committed r182351: <http://trac.webkit.org/changeset/182351> (In reply to comment #14) > Committed r182351: <http://trac.webkit.org/changeset/182351> It looks all ports build have broken since r182351. I upload a fix to Bug 143408. |