Bug 117836

<rdar://problem/14118228>

I think the problem is coming from HTMLCollection::namedItem() and this code right at the start of the method: ContainerNode* root = rootContainerNode(); if (name.isEmpty() || !root) return 0; This was added in http://trac.webkit.org/changeset/138195 to fix https://webkit.org/b/105324.

Cc'ing Antti who committed the change that changed the behaviour here. Looking at the spec at http://www.w3.org/TR/dom/#dom-htmlcollection-nameditem, I don't see why we would perform such a check, it seems perfectly valid to operate without a root: "The namedItem(key) method must return the first element in the collection that falls into one of the following categories: - It is an a, applet, area, embed, form, frame, frameset, iframe, img, or object element, in the HTML namespace, with a name attribute equal to key, or, - It is an element with an ID equal to key. If no such elements are found, then the method must return null."

Actually, the case we hit is the final "else" clause in this chunk of code: TreeScope* treeScope = root->treeScope(); Element* candidate = 0; if (treeScope->hasElementWithId(name.impl())) { if (!treeScope->containsMultipleElementsWithId(name)) candidate = treeScope->getElementById(name); } else if (treeScope->hasElementWithName(name.impl())) { if (!treeScope->containsMultipleElementsWithName(name)) { candidate = treeScope->getElementByName(name); if (candidate && type() == DocAll && (!candidate->isHTMLElement() || !nameShouldBeVisibleInDocumentAll(toHTMLElement(candidate)))) candidate = 0; } } else return 0; So the treeScope is not returning true for `treeScope->hasElementWithId(name.impl())`, which is surprising in this case.

Indeed, the treeScope's m_elementsById map is empty.

Element::updateIdForTreeScope() is where the map of element IDs is populated, and it's not yet been called at the time we're trying to access the namedItem in our test case. We need to be sure we have an ID map before calling namedItem(), although this may need to happen in other places too. Maybe this should happen in TreeScope::hasElementWithId().

Actually, this may have regressed with http://trac.webkit.org/changeset/149652 which fixed http://webkit.org/b/115584.

So the regression is indeed coming from http://trac.webkit.org/changeset/149652. The problem, I think, is that we don't get to register this id in the treeScope due to this early return in Element::insertedInto(): if (!insertionPoint->isInTreeScope()) return InsertionDone; As we hit this, we can't make the call to: updateIdForTreeScope(newScope, nullAtom, idValue); later on in the function. This needs to happen for the treeScope's m_elementsById to be populated with the expected elements when we eventually call HTMLCollection::namedItem().

If a node is not a descendent of the tree scope, then it shouldn't be in the hash map. Instead, we need to go through the slow path.

Created attachment 205119 [details] Fixes the bug

Taking it over.

Committed r151821: <http://trac.webkit.org/changeset/151821>