Bug 118504 - Introduce isSVGFontElement() and isSVGImageElement(), use them
Summary: Introduce isSVGFontElement() and isSVGImageElement(), use them
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gyuyoung Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-09 07:01 PDT by Gyuyoung Kim
Modified: 2013-07-10 00:11 PDT (History)
12 users (show)

See Also:


Attachments
Patch (12.28 KB, patch)
2013-07-09 07:05 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch for landing (12.06 KB, patch)
2013-07-09 21:16 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2013-07-09 07:01:14 PDT
Let's use isFoo() to enhance readibility in svg element classes along with html's.
Comment 1 Gyuyoung Kim 2013-07-09 07:05:02 PDT
Created attachment 206317 [details]
Patch
Comment 2 Benjamin Poulain 2013-07-09 20:29:00 PDT
Comment on attachment 206317 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=206317&action=review

> Source/WebCore/svg/SVGFontElement.h:95
> +    ASSERT_WITH_SECURITY_IMPLICATION(!node || isSVGFontElement(node));

How did that compile with isSVGFontElement() declared underneath?

> Source/WebCore/svg/SVGImageElement.h:94
> +inline bool isSVGImageElement(const Element* node)
> +{
> +    return node->hasTagName(SVGNames::imageTag);
> +}

Why? Element is a node. This should not be needed.
Comment 3 Gyuyoung Kim 2013-07-09 21:16:04 PDT
Created attachment 206365 [details]
 Patch for landing
Comment 4 Gyuyoung Kim 2013-07-09 21:19:53 PDT
(In reply to comment #2)
> > Source/WebCore/svg/SVGImageElement.h:94
> > +inline bool isSVGImageElement(const Element* node)
> > +{
> > +    return node->hasTagName(SVGNames::imageTag);
> > +}
> 
> Why? Element is a node. This should not be needed.

I confused that because some html classes added it. It looks we don't need to keep it at there as well.

http://trac.webkit.org/browser/trunk/Source/WebCore/html/HTMLAnchorElement.h#L162
Comment 5 WebKit Commit Bot 2013-07-09 22:32:44 PDT
Comment on attachment 206365 [details]
 Patch for landing

Clearing flags on attachment: 206365

Committed r152524: <http://trac.webkit.org/changeset/152524>
Comment 6 WebKit Commit Bot 2013-07-09 22:32:47 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Csaba Osztrogonác 2013-07-10 00:00:16 PDT
Comment on attachment 206365 [details]
 Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=206365&action=review

> Source/WebCore/svg/SVGImageElement.cpp:67
> -    ASSERT(hasTagName(SVGNames::imageTag));
> +    ASSERT(isSVGImageElement(SVGNames::imageTag));

It broke the debug builds:

/home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/Source/WebCore/svg/SVGImageElement.cpp: In constructor 'WebCore::SVGImageElement::SVGImageElement(const WebCore::QualifiedName&, WebCore::Document*)':
[1;31m/home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/Source/WebCore/svg/SVGImageElement.cpp:67:44: error: cannot convert 'const WebCore::QualifiedName' to 'const WebCore::Node*' for argument '1' to 'bool WebCore::isSVGImageElement(const WebCore::Node*)'[0m


Could you fix it, please?
Comment 8 Csaba Osztrogonác 2013-07-10 00:04:57 PDT
Fix already landed in http://trac.webkit.org/changeset/152526 ... but without any reference to this bug/commit and without any comment in this bug report :-/
Comment 9 Gyuyoung Kim 2013-07-10 00:08:17 PDT
(In reply to comment #8)
> Fix already landed in http://trac.webkit.org/changeset/152526 ... but without any reference to this bug/commit and without any comment in this bug report :-/

Sorry for inconvenience about it. I will let him know how to write ChangeLog for unreviewed patch.
Comment 10 Csaba Osztrogonác 2013-07-10 00:11:56 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > Fix already landed in http://trac.webkit.org/changeset/152526 ... but without any reference to this bug/commit and without any comment in this bug report :-/
> 
> Sorry for inconvenience about it. I will let him know how to write ChangeLog for unreviewed patch.

Thanks. Not a big problem, but a good changelog and a comment 
in the original bug helps a lot to avoid unnecessary requests :)