Reviewed by Anders Carlsson.
Bug 23106: HTMLFormCollection::namedItem ignores caseSensitive argument
https://bugs.webkit.org/show_bug.cgi?id=23106
This led me to a bunch of dead code. It turns out that HTML collections
were carrying the case-insensitive code just so they could be used to
find anchors, something we can do more simply and efficiently without
creating a DOM HTMLCollection object.
No behavior change. Just adding a new function findAnchor function and
removing some dead code.
* dom/Document.cpp:
(WebCore::Document::findAnchor): Added.
* dom/Document.h: Ditto.
* html/HTMLAnchorElement.cpp:
(WebCore::HTMLAnchorElement::accessKey): Take and return AtomicString
references for better efficiency.
(WebCore::HTMLAnchorElement::setAccessKey): Ditto.
(WebCore::HTMLAnchorElement::charset): Ditto.
(WebCore::HTMLAnchorElement::setCharset): Ditto.
(WebCore::HTMLAnchorElement::coords): Ditto.
(WebCore::HTMLAnchorElement::setCoords): Ditto.
(WebCore::HTMLAnchorElement::setHref): Ditto.
(WebCore::HTMLAnchorElement::hreflang): Ditto.
(WebCore::HTMLAnchorElement::setHreflang): Ditto.
(WebCore::HTMLAnchorElement::name): Ditto.
(WebCore::HTMLAnchorElement::setName): Ditto.
(WebCore::HTMLAnchorElement::rel): Ditto.
(WebCore::HTMLAnchorElement::setRel): Ditto.
(WebCore::HTMLAnchorElement::rev): Ditto.
(WebCore::HTMLAnchorElement::setRev): Ditto.
(WebCore::HTMLAnchorElement::shape): Ditto.
(WebCore::HTMLAnchorElement::setShape): Ditto.
(WebCore::HTMLAnchorElement::setTarget): Ditto.
(WebCore::HTMLAnchorElement::type): Ditto.
(WebCore::HTMLAnchorElement::setType): Ditto.
* html/HTMLAnchorElement.h: Ditto.
* html/HTMLCollection.cpp:
(WebCore::HTMLCollection::checkForNameMatch): Changed argument to an
AtomicString and removed the caseSensitive boolean, since we're now always
case sensitive.
(WebCore::HTMLCollection::namedItem): Ditto.
(WebCore::HTMLCollection::nextNamedItem): Ditto.
* html/HTMLCollection.h: Ditto.
* html/HTMLFormCollection.cpp:
(WebCore::HTMLFormCollection::getNamedItem): Ditto.
(WebCore::HTMLFormCollection::getNamedFormItem): Ditto.
(WebCore::HTMLFormCollection::nextNamedItemInternal): Ditto.
(WebCore::HTMLFormCollection::namedItem): Ditto.
(WebCore::HTMLFormCollection::nextNamedItem): Ditto.
* html/HTMLFormCollection.h: Ditto.
* html/HTMLSelectElement.cpp:
(WebCore::HTMLSelectElement::namedItem): Ditto.
* html/HTMLSelectElement.h: Ditto.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::gotoAnchor): Use the new findAnchor function.
* page/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::internalLinkElement): Ditto.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@39603
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
Reviewed by Anders Carlsson.
+ Bug 23106: HTMLFormCollection::namedItem ignores caseSensitive argument
+ https://bugs.webkit.org/show_bug.cgi?id=23106
+
+ This led me to a bunch of dead code. It turns out that HTML collections
+ were carrying the case-insensitive code just so they could be used to
+ find anchors, something we can do more simply and efficiently without
+ creating a DOM HTMLCollection object.
+
+ No behavior change. Just adding a new function findAnchor function and
+ removing some dead code.
+
+ * dom/Document.cpp:
+ (WebCore::Document::findAnchor): Added.
+ * dom/Document.h: Ditto.
+
+ * html/HTMLAnchorElement.cpp:
+ (WebCore::HTMLAnchorElement::accessKey): Take and return AtomicString
+ references for better efficiency.
+ (WebCore::HTMLAnchorElement::setAccessKey): Ditto.
+ (WebCore::HTMLAnchorElement::charset): Ditto.
+ (WebCore::HTMLAnchorElement::setCharset): Ditto.
+ (WebCore::HTMLAnchorElement::coords): Ditto.
+ (WebCore::HTMLAnchorElement::setCoords): Ditto.
+ (WebCore::HTMLAnchorElement::setHref): Ditto.
+ (WebCore::HTMLAnchorElement::hreflang): Ditto.
+ (WebCore::HTMLAnchorElement::setHreflang): Ditto.
+ (WebCore::HTMLAnchorElement::name): Ditto.
+ (WebCore::HTMLAnchorElement::setName): Ditto.
+ (WebCore::HTMLAnchorElement::rel): Ditto.
+ (WebCore::HTMLAnchorElement::setRel): Ditto.
+ (WebCore::HTMLAnchorElement::rev): Ditto.
+ (WebCore::HTMLAnchorElement::setRev): Ditto.
+ (WebCore::HTMLAnchorElement::shape): Ditto.
+ (WebCore::HTMLAnchorElement::setShape): Ditto.
+ (WebCore::HTMLAnchorElement::setTarget): Ditto.
+ (WebCore::HTMLAnchorElement::type): Ditto.
+ (WebCore::HTMLAnchorElement::setType): Ditto.
+ * html/HTMLAnchorElement.h: Ditto.
+
+ * html/HTMLCollection.cpp:
+ (WebCore::HTMLCollection::checkForNameMatch): Changed argument to an
+ AtomicString and removed the caseSensitive boolean, since we're now always
+ case sensitive.
+ (WebCore::HTMLCollection::namedItem): Ditto.
+ (WebCore::HTMLCollection::nextNamedItem): Ditto.
+ * html/HTMLCollection.h: Ditto.
+ * html/HTMLFormCollection.cpp:
+ (WebCore::HTMLFormCollection::getNamedItem): Ditto.
+ (WebCore::HTMLFormCollection::getNamedFormItem): Ditto.
+ (WebCore::HTMLFormCollection::nextNamedItemInternal): Ditto.
+ (WebCore::HTMLFormCollection::namedItem): Ditto.
+ (WebCore::HTMLFormCollection::nextNamedItem): Ditto.
+ * html/HTMLFormCollection.h: Ditto.
+ * html/HTMLSelectElement.cpp:
+ (WebCore::HTMLSelectElement::namedItem): Ditto.
+ * html/HTMLSelectElement.h: Ditto.
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::gotoAnchor): Use the new findAnchor function.
+ * page/AccessibilityRenderObject.cpp:
+ (WebCore::AccessibilityRenderObject::internalLinkElement): Ditto.
+
+2009-01-05 Darin Adler <darin@apple.com>
+
+ Reviewed by Anders Carlsson.
+
Bug 23104: minor mistakes in init functions for animation and transition events
https://bugs.webkit.org/show_bug.cgi?id=23104
#include "FrameLoader.h"
#include "FrameTree.h"
#include "FrameView.h"
+#include "HTMLAnchorElement.h"
#include "HTMLBodyElement.h"
#include "HTMLCanvasElement.h"
#include "HTMLDocument.h"
}
}
+Element* Document::findAnchor(const String& name)
+{
+ if (name.isEmpty())
+ return 0;
+ if (Element* element = getElementById(name))
+ return element;
+ for (Node* node = this; node; node = traverseNextNode()) {
+ if (node->hasTagName(aTag)) {
+ HTMLAnchorElement* anchor = static_cast<HTMLAnchorElement*>(node);
+ if (inCompatMode()) {
+ // Quirks mode, case insensitive comparison of names.
+ if (equalIgnoringCase(anchor->name(), name))
+ return anchor;
+ } else {
+ // Strict mode, names need to match exactly.
+ if (anchor->name() == name)
+ return anchor;
+ }
+ }
+ }
+ return 0;
+}
+
} // namespace WebCore
PassRefPtr<HTMLCollection> windowNamedItems(const String& name);
PassRefPtr<HTMLCollection> documentNamedItems(const String& name);
+ // Find first anchor with the given name.
+ // First searches for an element with the given ID, but if that fails, then looks
+ // for an anchor with the given name. ID matching is always case sensitive, but
+ // Anchor name matching is case sensitive in strict mode and not case sensitive in
+ // quirks mode for historical compatibility reasons.
+ Element* findAnchor(const String& name);
+
HTMLCollection::CollectionInfo* collectionInfo(HTMLCollection::Type type)
{
ASSERT(type >= HTMLCollection::FirstUnnamedDocumentCachedType);
return isContentEditable();
}
-String HTMLAnchorElement::accessKey() const
+const AtomicString& HTMLAnchorElement::accessKey() const
{
return getAttribute(accesskeyAttr);
}
-void HTMLAnchorElement::setAccessKey(const String &value)
+void HTMLAnchorElement::setAccessKey(const AtomicString& value)
{
setAttribute(accesskeyAttr, value);
}
-String HTMLAnchorElement::charset() const
+const AtomicString& HTMLAnchorElement::charset() const
{
return getAttribute(charsetAttr);
}
-void HTMLAnchorElement::setCharset(const String &value)
+void HTMLAnchorElement::setCharset(const AtomicString& value)
{
setAttribute(charsetAttr, value);
}
-String HTMLAnchorElement::coords() const
+const AtomicString& HTMLAnchorElement::coords() const
{
return getAttribute(coordsAttr);
}
-void HTMLAnchorElement::setCoords(const String &value)
+void HTMLAnchorElement::setCoords(const AtomicString& value)
{
setAttribute(coordsAttr, value);
}
return document()->completeURL(getAttribute(hrefAttr));
}
-void HTMLAnchorElement::setHref(const String &value)
+void HTMLAnchorElement::setHref(const AtomicString& value)
{
setAttribute(hrefAttr, value);
}
-String HTMLAnchorElement::hreflang() const
+const AtomicString& HTMLAnchorElement::hreflang() const
{
return getAttribute(hreflangAttr);
}
-void HTMLAnchorElement::setHreflang(const String &value)
+void HTMLAnchorElement::setHreflang(const AtomicString& value)
{
setAttribute(hreflangAttr, value);
}
-String HTMLAnchorElement::name() const
+const AtomicString& HTMLAnchorElement::name() const
{
return getAttribute(nameAttr);
}
-void HTMLAnchorElement::setName(const String &value)
+void HTMLAnchorElement::setName(const AtomicString& value)
{
setAttribute(nameAttr, value);
}
-String HTMLAnchorElement::rel() const
+const AtomicString& HTMLAnchorElement::rel() const
{
return getAttribute(relAttr);
}
-void HTMLAnchorElement::setRel(const String &value)
+void HTMLAnchorElement::setRel(const AtomicString& value)
{
setAttribute(relAttr, value);
}
-String HTMLAnchorElement::rev() const
+const AtomicString& HTMLAnchorElement::rev() const
{
return getAttribute(revAttr);
}
-void HTMLAnchorElement::setRev(const String &value)
+void HTMLAnchorElement::setRev(const AtomicString& value)
{
setAttribute(revAttr, value);
}
-String HTMLAnchorElement::shape() const
+const AtomicString& HTMLAnchorElement::shape() const
{
return getAttribute(shapeAttr);
}
-void HTMLAnchorElement::setShape(const String &value)
+void HTMLAnchorElement::setShape(const AtomicString& value)
{
setAttribute(shapeAttr, value);
}
return getAttribute(targetAttr);
}
-void HTMLAnchorElement::setTarget(const String &value)
+void HTMLAnchorElement::setTarget(const AtomicString& value)
{
setAttribute(targetAttr, value);
}
-String HTMLAnchorElement::type() const
+const AtomicString& HTMLAnchorElement::type() const
{
return getAttribute(typeAttr);
}
-void HTMLAnchorElement::setType(const String &value)
+void HTMLAnchorElement::setType(const AtomicString& value)
{
setAttribute(typeAttr, value);
}
virtual bool canStartSelection() const;
- String accessKey() const;
- void setAccessKey(const String&);
+ const AtomicString& accessKey() const;
+ void setAccessKey(const AtomicString&);
- String charset() const;
- void setCharset(const String&);
+ const AtomicString& charset() const;
+ void setCharset(const AtomicString&);
- String coords() const;
- void setCoords(const String&);
+ const AtomicString& coords() const;
+ void setCoords(const AtomicString&);
KURL href() const;
- void setHref(const String&);
+ void setHref(const AtomicString&);
- String hreflang() const;
- void setHreflang(const String&);
+ const AtomicString& hreflang() const;
+ void setHreflang(const AtomicString&);
- String name() const;
- void setName(const String&);
+ const AtomicString& name() const;
+ void setName(const AtomicString&);
- String rel() const;
- void setRel(const String&);
+ const AtomicString& rel() const;
+ void setRel(const AtomicString&);
- String rev() const;
- void setRev(const String&);
+ const AtomicString& rev() const;
+ void setRev(const AtomicString&);
- String shape() const;
- void setShape(const String&);
+ const AtomicString& shape() const;
+ void setShape(const AtomicString&);
virtual short tabIndex() const;
virtual String target() const;
- void setTarget(const String&);
+ void setTarget(const AtomicString&);
- String type() const;
- void setType(const String&);
+ const AtomicString& type() const;
+ void setType(const AtomicString&);
String hash() const;
String host() const;
return retval;
}
-bool HTMLCollection::checkForNameMatch(Element* element, bool checkName, const String& name, bool caseSensitive) const
+bool HTMLCollection::checkForNameMatch(Element* element, bool checkName, const AtomicString& name) const
{
if (!element->isHTMLElement())
return false;
HTMLElement* e = static_cast<HTMLElement*>(element);
- if (caseSensitive) {
- if (checkName) {
- // document.all returns only images, forms, applets, objects and embeds
- // by name (though everything by id)
- if (m_type == DocAll &&
- !(e->hasLocalName(imgTag) || e->hasLocalName(formTag) ||
- e->hasLocalName(appletTag) || e->hasLocalName(objectTag) ||
- e->hasLocalName(embedTag) || e->hasLocalName(inputTag) ||
- e->hasLocalName(selectTag)))
- return false;
-
- return e->getAttribute(nameAttr) == name && e->getAttribute(idAttr) != name;
- } else
- return e->getAttribute(idAttr) == name;
- } else {
- if (checkName) {
- // document.all returns only images, forms, applets, objects and embeds
- // by name (though everything by id)
- if (m_type == DocAll &&
- !(e->hasLocalName(imgTag) || e->hasLocalName(formTag) ||
- e->hasLocalName(appletTag) || e->hasLocalName(objectTag) ||
- e->hasLocalName(embedTag) || e->hasLocalName(inputTag) ||
- e->hasLocalName(selectTag)))
- return false;
-
- return equalIgnoringCase(e->getAttribute(nameAttr), name)
- && !equalIgnoringCase(e->getAttribute(idAttr), name);
- } else
- return equalIgnoringCase(e->getAttribute(idAttr), name);
- }
-}
+ if (!checkName)
+ return e->getAttribute(idAttr) == name;
+
+ // document.all returns only images, forms, applets, objects and embeds
+ // by name (though everything by id)
+ if (m_type == DocAll &&
+ !(e->hasLocalName(imgTag) || e->hasLocalName(formTag) ||
+ e->hasLocalName(appletTag) || e->hasLocalName(objectTag) ||
+ e->hasLocalName(embedTag) || e->hasLocalName(inputTag) ||
+ e->hasLocalName(selectTag)))
+ return false;
+ return e->getAttribute(nameAttr) == name && e->getAttribute(idAttr) != name;
+}
-Node *HTMLCollection::namedItem(const String &name, bool caseSensitive) const
+Node* HTMLCollection::namedItem(const AtomicString& name) const
{
// http://msdn.microsoft.com/workshop/author/dhtml/reference/methods/nameditem.asp
// This method first searches for an object with a matching id
m_idsDone = false;
for (Element* e = itemAfter(0); e; e = itemAfter(e)) {
- if (checkForNameMatch(e, m_idsDone, name, caseSensitive)) {
+ if (checkForNameMatch(e, m_idsDone, name)) {
m_info->current = e;
return e;
}
m_idsDone = true;
for (Element* e = itemAfter(0); e; e = itemAfter(e)) {
- if (checkForNameMatch(e, m_idsDone, name, caseSensitive)) {
+ if (checkForNameMatch(e, m_idsDone, name)) {
m_info->current = e;
return e;
}
}
-Node* HTMLCollection::nextNamedItem(const String& name) const
+Node* HTMLCollection::nextNamedItem(const AtomicString& name) const
{
resetCollectionInfo();
for (Element* e = itemAfter(m_info->current); e; e = itemAfter(e)) {
- if (checkForNameMatch(e, m_idsDone, name, true)) {
+ if (checkForNameMatch(e, m_idsDone, name)) {
m_info->current = e;
return e;
}
m_idsDone = true;
for (Element* e = itemAfter(m_info->current); e; e = itemAfter(e)) {
- if (checkForNameMatch(e, m_idsDone, name, true)) {
+ if (checkForNameMatch(e, m_idsDone, name)) {
m_info->current = e;
return e;
}
virtual Node* item(unsigned index) const;
virtual Node* nextItem() const;
- virtual Node* namedItem(const String& name, bool caseSensitive = true) const;
- virtual Node* nextNamedItem(const String& name) const; // In case of multiple items named the same way
+ virtual Node* namedItem(const AtomicString& name) const;
+ virtual Node* nextNamedItem(const AtomicString& name) const; // In case of multiple items named the same way
Node* firstItem() const;
virtual unsigned calcLength() const;
virtual void updateNameCache() const;
- bool checkForNameMatch(Element*, bool checkName, const String &name, bool caseSensitive) const;
+ bool checkForNameMatch(Element*, bool checkName, const AtomicString& name) const;
RefPtr<Node> m_base;
Type m_type;
return 0;
}
-Element* HTMLFormCollection::getNamedItem(const QualifiedName& attrName, const String& name, bool caseSensitive) const
+Element* HTMLFormCollection::getNamedItem(const QualifiedName& attrName, const AtomicString& name) const
{
info()->position = 0;
- return getNamedFormItem(attrName, name, 0, caseSensitive);
+ return getNamedFormItem(attrName, name, 0);
}
-Element* HTMLFormCollection::getNamedFormItem(const QualifiedName& attrName, const String& name, int duplicateNumber, bool caseSensitive) const
+Element* HTMLFormCollection::getNamedFormItem(const QualifiedName& attrName, const String& name, int duplicateNumber) const
{
HTMLFormElement* form = static_cast<HTMLFormElement*>(base());
bool foundInputElements = false;
for (unsigned i = 0; i < form->formElements.size(); ++i) {
HTMLFormControlElement* e = form->formElements[i];
- if (e->isEnumeratable()) {
- bool found;
- if (caseSensitive)
- found = e->getAttribute(attrName) == name;
- else
- found = equalIgnoringCase(e->getAttribute(attrName), name);
- if (found) {
- foundInputElements = true;
- if (!duplicateNumber)
- return e;
- --duplicateNumber;
- }
+ if (e->isEnumeratable() && e->getAttribute(attrName) == name) {
+ foundInputElements = true;
+ if (!duplicateNumber)
+ return e;
+ --duplicateNumber;
}
}
if (!foundInputElements) {
for (unsigned i = 0; i < form->imgElements.size(); ++i) {
HTMLImageElement* e = form->imgElements[i];
- bool found;
- if (caseSensitive)
- found = e->getAttribute(attrName) == name;
- else
- found = equalIgnoringCase(e->getAttribute(attrName), name);
- if (found) {
+ if (e->getAttribute(attrName) == name) {
if (!duplicateNumber)
return e;
--duplicateNumber;
Element* HTMLFormCollection::nextNamedItemInternal(const String &name) const
{
- Element* retval = getNamedFormItem(m_idsDone ? nameAttr : idAttr, name, ++info()->position, true);
+ Element* retval = getNamedFormItem(m_idsDone ? nameAttr : idAttr, name, ++info()->position);
if (retval)
return retval;
if (m_idsDone) // we're done
return 0;
// After doing id, do name
m_idsDone = true;
- return getNamedItem(nameAttr, name, true);
+ return getNamedItem(nameAttr, name);
}
-Node* HTMLFormCollection::namedItem(const String& name, bool caseSensitive) const
+Node* HTMLFormCollection::namedItem(const AtomicString& name) const
{
// http://msdn.microsoft.com/workshop/author/dhtml/reference/methods/nameditem.asp
// This method first searches for an object with a matching id
// that are allowed a name attribute.
resetCollectionInfo();
m_idsDone = false;
- info()->current = getNamedItem(idAttr, name, true);
+ info()->current = getNamedItem(idAttr, name);
if (info()->current)
return info()->current;
m_idsDone = true;
- info()->current = getNamedItem(nameAttr, name, true);
+ info()->current = getNamedItem(nameAttr, name);
return info()->current;
}
-Node* HTMLFormCollection::nextNamedItem(const String& name) const
+Node* HTMLFormCollection::nextNamedItem(const AtomicString& name) const
{
// The nextNamedItemInternal function can return the same item twice if it has
// both an id and name that are equal to the name parameter. So this function
virtual Node* item(unsigned index) const;
virtual Node* nextItem() const;
- virtual Node* namedItem(const String& name, bool caseSensitive = true) const;
- virtual Node* nextNamedItem(const String& name) const;
+ virtual Node* namedItem(const AtomicString& name) const;
+ virtual Node* nextNamedItem(const AtomicString& name) const;
private:
HTMLFormCollection(PassRefPtr<HTMLFormElement>);
static CollectionInfo* formCollectionInfo(HTMLFormElement*);
- Element* getNamedItem(const QualifiedName& attrName, const String& name, bool caseSensitive) const;
+ Element* getNamedItem(const QualifiedName& attrName, const AtomicString& name) const;
Element* nextNamedItemInternal(const String& name) const;
- Element* getNamedFormItem(const QualifiedName& attrName, const String& name, int duplicateNumber, bool caseSensitive) const;
+ Element* getNamedFormItem(const QualifiedName& attrName, const String& name, int duplicateNumber) const;
mutable int currentPos;
};
setAttribute(sizeAttr, String::number(size));
}
-Node* HTMLSelectElement::namedItem(const String &name, bool caseSensitive)
+Node* HTMLSelectElement::namedItem(const AtomicString& name)
{
- return options()->namedItem(name, caseSensitive);
+ return options()->namedItem(name);
}
Node* HTMLSelectElement::item(unsigned index)
void setOption(unsigned index, HTMLOptionElement*, ExceptionCode&);
void setLength(unsigned, ExceptionCode&);
- Node* namedItem(const String& name, bool caseSensitive = true);
+ Node* namedItem(const AtomicString& name);
Node* item(unsigned index);
HTMLCollection::CollectionInfo* collectionInfo() { return &m_collectionInfo; }
#include "FramePrivate.h"
#include "FrameTree.h"
#include "FrameView.h"
+#include "HTMLAnchorElement.h"
#include "HTMLFormElement.h"
#include "HTMLFrameElement.h"
#include "HTMLNames.h"
m_frame->document()->setGotoAnchorNeededAfterStylesheetsLoad(false);
- Node* anchorNode = m_frame->document()->getElementById(AtomicString(name));
- if (!anchorNode && !name.isEmpty())
- anchorNode = m_frame->document()->anchors()->namedItem(name, !m_frame->document()->inCompatMode());
+ Element* anchorNode = m_frame->document()->findAnchor(name);
#if ENABLE(SVG)
if (m_frame->document()->isSVGDocument()) {
if (m_renderer->document()->url() != linkURL)
return 0;
- Node* linkedNode = m_renderer->document()->getElementById(ref);
- if (!linkedNode) {
- linkedNode = m_renderer->document()->anchors()->namedItem(ref, !m_renderer->document()->inCompatMode());
- if (!linkedNode)
- return 0;
- }
+ Node* linkedNode = m_renderer->document()->findAnchor(ref);
+ if (!linkedNode)
+ return 0;
// the element we find may not be accessible, keep searching until we find a good one
AccessibilityObject* linkedAXElement = m_renderer->document()->axObjectCache()->get(linkedNode->renderer());