Issue 195813003: Use new is*Element() helper functions further more in HTML code

Issue 195813003: Use new is*Element() helper functions further more in HTML code (Closed)

Created:
6 years, 9 months ago by Inactive

Modified:
6 years, 9 months ago

Reviewers:
philipj_slow, adamk, eseidel

CC:
blink-reviews, kenneth.christiansen, nessy, philipj_slow, gasubic, fs, eric.carlson_apple.com, feature-media-reviews_chromium.org, dglazkov+blink, adamk+blink_chromium.org, vcarbune.chromium

Base URL:
svn://svn.chromium.org/blink/trunk

Visibility:
Public.

More Reviews

Description

Use new is*Element() helper functions further more in HTML code Use new is*Element() helpers further more in HTML code. This makes the code clearer and simpler. Also use the new Traversal<*Element> API when suitable. R=eseidel, adamk BUG=346095 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169168

Patch Set 1 #

Patch Set 2 : Fix bad assertion #

Total comments: 3

Created: 6 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+91 lines, -102 lines)			Patch
M	Source/core/html/HTMLLIElement.cpp	View		1 chunk	+1 line, -1 line	0 comments	Download
M	Source/core/html/HTMLLegendElement.cpp	View		2 chunks	+2 lines, -2 lines	0 comments	Download
M	Source/core/html/HTMLMapElement.cpp	View	1	1 chunk	+4 lines, -5 lines	1 comment	Download
M	Source/core/html/HTMLMediaElement.cpp	View		5 chunks	+7 lines, -17 lines	2 comments	Download
M	Source/core/html/HTMLMetaElement-in.cpp	View		1 chunk	+1 line, -1 line	0 comments	Download
M	Source/core/html/HTMLNameCollection.cpp	View		2 chunks	+11 lines, -11 lines	0 comments	Download
M	Source/core/html/HTMLObjectElement.cpp	View		3 chunks	+9 lines, -8 lines	0 comments	Download
M	Source/core/html/HTMLOptGroupElement.cpp	View		2 chunks	+2 lines, -2 lines	0 comments	Download
M	Source/core/html/HTMLOptionElement.cpp	View		5 chunks	+5 lines, -5 lines	0 comments	Download
M	Source/core/html/HTMLOptionsCollection.cpp	View		1 chunk	+1 line, -1 line	0 comments	Download
M	Source/core/html/HTMLSelectElement.cpp	View		22 chunks	+32 lines, -31 lines	0 comments	Download
M	Source/core/html/HTMLSummaryElement.cpp	View		1 chunk	+1 line, -1 line	0 comments	Download
M	Source/core/html/HTMLTableCellElement.cpp	View		1 chunk	+4 lines, -6 lines	0 comments	Download
M	Source/core/html/HTMLTableElement.cpp	View		2 chunks	+2 lines, -1 line	0 comments	Download
M	Source/core/html/HTMLTablePartElement.cpp	View		1 chunk	+1 line, -1 line	0 comments	Download
M	Source/core/html/HTMLTableRowElement.cpp	View		2 chunks	+5 lines, -6 lines	0 comments	Download
M	Source/core/html/HTMLTextFormControlElement.cpp	View		3 chunks	+3 lines, -3 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

adamk

lgtm https://codereview.chromium.org/195813003/diff/20001/Source/core/html/HTMLMapElement.cpp File Source/core/html/HTMLMapElement.cpp (right): https://codereview.chromium.org/195813003/diff/20001/Source/core/html/HTMLMapElement.cpp#newcode77 Source/core/html/HTMLMapElement.cpp:77: ASSERT(isHTMLImageElement(curr)); I take it this is just asserting ...

6 years, 9 months ago (2014-03-13 20:20:35 UTC) #2

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/195813003/20001

6 years, 9 months ago (2014-03-13 20:20:39 UTC) #3

philipj_slow

6 years, 9 months ago (2014-03-14 02:20:00 UTC) #6

Message was sent while issue was closed.

https://codereview.chromium.org/195813003/diff/20001/Source/core/html/HTMLMed...
File Source/core/html/HTMLMediaElement.cpp (left):

https://codereview.chromium.org/195813003/diff/20001/Source/core/html/HTMLMed...
Source/core/html/HTMLMediaElement.cpp:2322:
ASSERT(trackElement->hasTagName(trackTag));
On 2014/03/13 20:20:36, adamk wrote:
> Do you understand why this was here before? I certainly don't.

It's from <http://trac.webkit.org/changeset/102968>. At the time the feature was
controlled by an ifdef so maybe it seemed more plausible that this could end up
accidentally being called with the feature disabled. Doesn't matter now.

Expand Messages | Collapse Messages