Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(608)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2628873003: Look for favicon URLs (and similar <link>s) in SVG documents (Closed)

Created:
3 years, 11 months ago by fs
Modified:
3 years, 11 months ago
Reviewers:
pdr.
CC:
blink-reviews, blink-reviews-dom_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, rwlbuis, sof
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Look for favicon URLs (and similar <link>s) in SVG documents Previously we were only looking for <link>s with icons within <head>, and since SVG document don't have those, no icons would be found. Instead, for SVG documents (with a root/document element of <svg>), just collect all <link> (HTMLLinkElement) elements, regardless of position in the document, using a pre-order traversal. This appears to match the behavior of Gecko. BUG=385466 Review-Url: https://codereview.chromium.org/2628873003 Cr-Commit-Position: refs/heads/master@{#443336} Committed: https://chromium.googlesource.com/chromium/src/+/2546ba44189a370193e64d005aa554bd44490e1d

Patch Set 1 #

Patch Set 2 : Find all <link>s in document (like Gecko.) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+47 lines, -5 lines) Patch
A third_party/WebKit/LayoutTests/svg/custom/favicon-link.svg View 1 1 chunk +30 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/svg/custom/favicon-link-expected.txt View 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Document.cpp View 1 1 chunk +15 lines, -5 lines 0 comments Download

Messages

Total messages: 21 (14 generated)
fs
Gecko appears to just look through all the <link> elements. I guess we could do ...
3 years, 11 months ago (2017-01-11 17:14:39 UTC) #4
pdr.
On 2017/01/11 at 17:14:39, fs wrote: > Gecko appears to just look through all the ...
3 years, 11 months ago (2017-01-12 00:04:33 UTC) #7
fs
On 2017/01/12 at 00:04:33, pdr wrote: > On 2017/01/11 at 17:14:39, fs wrote: > > ...
3 years, 11 months ago (2017-01-12 11:56:25 UTC) #10
pdr.
On 2017/01/12 at 11:56:25, fs wrote: > On 2017/01/12 at 00:04:33, pdr wrote: > > ...
3 years, 11 months ago (2017-01-12 19:54:33 UTC) #13
fs
On 2017/01/12 at 19:54:33, pdr wrote: > On 2017/01/12 at 11:56:25, fs wrote: > > ...
3 years, 11 months ago (2017-01-12 19:59:36 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2628873003/20001
3 years, 11 months ago (2017-01-12 20:06:09 UTC) #18
commit-bot: I haz the power
3 years, 11 months ago (2017-01-12 20:14:44 UTC) #21
Message was sent while issue was closed. 
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/2546ba44189a370193e64d005aa5...

Powered by Google App Engine
This is Rietveld 408576698