[Webkit-unassigned] [Bug 137860] Add relList to the anchor, area and link elements

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 20 11:32:46 PDT 2014


https://bugs.webkit.org/show_bug.cgi?id=137860

--- Comment #9 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 240100
  --> https://bugs.webkit.org/attachment.cgi?id=240100
Patch

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

>> Source/WebCore/html/HTMLAnchorElement.cpp:319
>> +        m_relList->setRel(value);
> 
> There’s a small unnecessary inefficiency here. This function takes a const String&, but the setRel function takes a const AtomicString&. That means that this call needs to construct an AtomicString from a String. This means we will be compiling in a check to see if a string is already in the atomic string table. All of that can be avoided two different ways:
> 
> a) Change the argument type for this function to const AtomicString&.
> b) Move the code from here to the single call site, and get rid of this function entirely.
> 
> I think the name of this function is misleading, because it does not *set* the rel attribute. It simply responds to a change in the rel attribute.
> 
> So to summarize:
> 
> - This function’s name is not clear.
> - This function’s implementation is slightly inefficient in an unnecessary way because of its argument type.
> - This function is short and called in only one place.
> - This function was public, but should be a completely private part of this class.
> 
> I suggest getting rid of it to fix all of these.

a) seems like a good idea because this function is only called from HTMLAnchorElement::parseAttribute() and always with an AtomicString.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141020/112a666a/attachment-0002.html>


More information about the webkit-unassigned mailing list