Bug 84791 – REGRESSION(r115099): html5lib/runner.html crashes.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Andreas Kling]

Assertion below WebCore::HTMLTreeBuilder::attributesForIsindexInput().

[Andreas Kling]

Created attachment 138671 [details]
Patch

[Andreas Kling]

Committed r115127: <http://trac.webkit.org/changeset/115127>

[Darin Adler]

Comment on attachment 138671 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        It's perfectly safe to remove elements from a Vector while iterating over it.

Sure, it’s safe if you do it correctly.

> Source/WebCore/html/parser/HTMLTreeBuilder.cpp:571
>      for (unsigned i = 0; i < attributes.size(); ++i) {
>          const QualifiedName& name = attributes.at(i).name();
>          if (name.matches(nameAttr) || name.matches(actionAttr) || name.matches(promptAttr))
> -            indicesToRemove.append(i);
> +            attributes.remove(i);
>      }

If two attributes in a row are name and action, this will return only name, but not action.

You need to subtract one from i when you remove the element at i so we process the new attribute that is now at index i.

[Andreas Kling]

(In reply to comment #3)
> (From update of attachment 138671 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=138671&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        It's perfectly safe to remove elements from a Vector while iterating over it.
> 
> Sure, it’s safe if you do it correctly.

Yeah, that was a silly mistake. I should know better than to commit when I'm sleepy. :(
I ended up rolling the whole thing out so it can be relanded by the original author without piling patches on top.

[Darin Adler]

(In reply to comment #4)
> I ended up rolling the whole thing out so it can be relanded by the original author without piling patches on top.

Sounds good.