Bug 88632 – REGRESSION(r116487?): HTMLFormElement::elements['name'] is empty if the form is detached from the document tree

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Kent Tamura]

http://code.google.com/p/chromium/issues/detail?id=131575

[Rakesh]

will check this issue.

[Rakesh]

Created attachment 146569 [details]
patch1

Root node of radionode list needs to updated to form if detached from dom tree

[Ryosuke Niwa]

Comment on attachment 146569 [details]
patch1

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

Please fix nits in the test before you land.

> Source/WebCore/dom/Node.cpp:2941
> +    ASSERT(hasTagName(formTag));
> +    if (!hasRareData() || !rareData()->nodeLists())
> +        return;
> +
> +    NodeListsNodeData::RadioNodeListCache cache = rareData()->nodeLists()->m_radioNodeListCache;
> +    for (NodeListsNodeData::RadioNodeListCache::iterator it = cache.begin(); it != cache.end(); ++it)
> +        it->second->setRootElement(toElement(this));

It seems like we have the same problem in LabelsNodeList and others that can be rooted at the document node.
We might want to fix them as well in a follow up patch.

> LayoutTests/fast/forms/radionodelist-whose-form-element-detached-from-domtree.html:10
> +    <input type="radio" name="type1" value="rbarrval1">

Why is the value "rbarrval1"? Can we use some other human-understandable value instead?

> LayoutTests/fast/forms/radionodelist-whose-form-element-detached-from-domtree.html:18
> +    <input type="checkbox" name="cb" value="cbval1">

Ditto.

[Rakesh]

Created attachment 146634 [details]
patch_for_landing

Fixed nits suggested.

[Rakesh]

(In reply to comment #3)
Thanks for reviewing this patch.
> 
> It seems like we have the same problem in LabelsNodeList and others that can be rooted at the document node.
> We might want to fix them as well in a follow up patch.
> 
Will check those.

> Why is the value "rbarrval1"? Can we use some other human-understandable value instead?
> 
Fixed.

[Ryosuke Niwa]

Comment on attachment 146634 [details]
patch_for_landing

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

> LayoutTests/fast/forms/radionodelist-whose-form-element-detached-from-domtree-expected.txt:10
> +PASS radioNodeList1[0].value is 'val1'

Please spell out value.

[Rakesh]

Created attachment 146647 [details]
patch

Better element values

[WebKit Review Bot]

Comment on attachment 146647 [details]
patch

Rejecting attachment 146647 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/12918649

[WebKit Review Bot]

Comment on attachment 146647 [details]
patch

Clearing flags on attachment: 146647

Committed r119891: <http://trac.webkit.org/changeset/119891>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Rakesh]

Thanks rniwa, I did not see that commit failed, I was not available, it was 2AM already.

[Ryosuke Niwa]

On my second thought, this patch doesn't fix the bug completely in some edge cases.

[Ryosuke Niwa]

I'm rolling out the patch because the fix is incorrect.

[Ryosuke Niwa]

Apparently sheriffbot can't roll out this patch. I'll file a new bug and manually revert this change and make the right fix.