Bug 57404

Summary:

Teach the preload scanner about <input type=image>

Product:

WebKit

Reporter:

Tony Gentilcore <tonyg>

Component:

New Bugs

Assignee:

Tony Gentilcore <tonyg>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, commit-queue, koivisto, simonjam

Priority:

Version:

528+ (Nightly build)

Hardware:

Other

OS:

OS X 10.5

Attachments:

Description	Flags
Patch	none
Patch for landing	none

Tony Gentilcore

Reported 2011-03-29 16:59:11 PDT

Teach the preload scanner about <input type=image>

Attachments
Patch (6.22 KB, patch) 2011-03-29 17:05 PDT, Tony Gentilcore	no flags	Details Formatted Diff Diff
Patch for landing (6.07 KB, patch) 2011-03-29 18:04 PDT, Tony Gentilcore	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Tony Gentilcore

Comment 1 2011-03-29 17:05:23 PDT

Created attachment 87436 [details] Patch

James Simonsen

Comment 2 2011-03-29 17:44:57 PDT

Comment on attachment 87436 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=87436&action=review LGTM > Source/WebCore/html/parser/HTMLPreloadScanner.cpp:101 > + static bool inputTypeAttributeIsImage(const String& attributeValue) I'd just inline this. The others are multiple lines, so it helps readability. Here, I think it'd be just as readable if this was inline.

Adam Barth

Comment 3 2011-03-29 17:58:49 PDT

Comment on attachment 87436 [details] Patch I agree with James.

Tony Gentilcore

Comment 4 2011-03-29 18:04:42 PDT

Created attachment 87446 [details] Patch for landing

Tony Gentilcore

Comment 5 2011-03-29 18:04:51 PDT

> I'd just inline this. The others are multiple lines, so it helps readability. Here, I think it'd be just as readable if this was inline. Good call. Done.

Darin Adler

Comment 6 2011-03-29 18:14:03 PDT

Comment on attachment 87436 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=87436&action=review > Source/WebCore/html/parser/HTMLPreloadScanner.cpp:64 > - if (m_tagName != scriptTag && m_tagName != imgTag && m_tagName != linkTag) > + if (m_tagName != imgTag > + && m_tagName != inputTag > + && m_tagName != linkTag > + && m_tagName != scriptTag) Seems like this would still fit fine all on one line.

Tony Gentilcore

Comment 7 2011-03-29 18:16:25 PDT

(In reply to comment #6) > (From update of attachment 87436 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=87436&action=review > > > Source/WebCore/html/parser/HTMLPreloadScanner.cpp:64 > > - if (m_tagName != scriptTag && m_tagName != imgTag && m_tagName != linkTag) > > + if (m_tagName != imgTag > > + && m_tagName != inputTag > > + && m_tagName != linkTag > > + && m_tagName != scriptTag) > > Seems like this would still fit fine all on one line. I am planning to add iframe as well so I broke it up and alphabetized. If you prefer I could leave it as-is for now. Don't worry, I understand not to over line break conditionals in general.

WebKit Commit Bot

Comment 8 2011-03-29 18:36:09 PDT

Comment on attachment 87446 [details] Patch for landing Clearing flags on attachment: 87446 Committed r82379: <http://trac.webkit.org/changeset/82379>

WebKit Commit Bot

Comment 9 2011-03-29 18:36:15 PDT

All reviewed patches have been landed. Closing bug.

Tony Gentilcore

Comment 10 2011-03-29 18:36:55 PDT

(In reply to comment #7) > (In reply to comment #6) > > (From update of attachment 87436 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=87436&action=review > > > > > Source/WebCore/html/parser/HTMLPreloadScanner.cpp:64 > > > - if (m_tagName != scriptTag && m_tagName != imgTag && m_tagName != linkTag) > > > + if (m_tagName != imgTag > > > + && m_tagName != inputTag > > > + && m_tagName != linkTag > > > + && m_tagName != scriptTag) > > > > Seems like this would still fit fine all on one line. > > I am planning to add iframe as well so I broke it up and alphabetized. If you prefer I could leave it as-is for now. Don't worry, I understand not to over line break conditionals in general. The cq landed this. I can go back and undo this change if you like.

Note You need to log in before you can comment on or make changes to this bug.