100175 - innerHTML smushing multiple options together on select

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect, P3)

Description

[John Keiser (jkeiser)]

When you set innerHTML = "<option>...<option>..." it smushes the options
together into one big option and adds that to <select>.  This is consistent and
reproducible on 0.9.4 and beyond (haven't tried earlier).

Relevant JavaScript:sel.innerHTML = "<OPTION VALUE='A'>A</OPTION><OPTION
VALUE='B'>B</OPTION>";

Removing </OPTION> doesn't fix the problem.

It would appear that this is a bug in innerHTML or in adding HTML to a tree
rather than <select> itself; adding individual options, it should be noted,
works fine: http://www.johnkeiser.com/mozilla/select/jsadd.html.

Comment 1

[Johnny Stenback (:jst)]

jkeiser, I can't access the testcase in this bug, is it still available
somewhere? Is this problem still around?

Comment 2

[John Keiser (jkeiser)]

Oops, it's moved to a bigger regression test page.  Setting URL.  Click
"innerHTML =" button on one of the selects.  It's still acting the same for me
nowm showing "new0new1" for example, the first time you click.

Note that the smushed-up option can't be selected either.  That might actually
be a clue.

Comment 3

[Johnny Stenback (:jst)]

The parser doesn't get this right, it ends up adding one single textnode in
stead of adding two options to the select when innerHTML is set to
<option>...</option><option>...</option>. Over to the parser component.

Comment 4

[Johnny Stenback (:jst)]

Attachment: Whack the parser to not loose the <endnote> tag

Comment 5

[Johnny Stenback (:jst)]

Harish, does this look reasonable or is there a better way to do this?

Comment 6

[harishd]

jst: It should be possible to tweak the element table instead. I just don't like
the idea of calling OpenContainer() abruptly :)

Comment 7

[harishd]

mass move to 1.1

Comment 8

[Heikki Toivonen (remove -bugzilla when emailing directly)]

[was 1.1alpha]

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Allowing <endnote> anywhere fixes this bug, but I'm not entirely sure what
<endnote> is at the moment. I'll attach a patch once my tree is clear from bug
88952.

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: whack the parser in a friendlier way

This is basically the same thing as jst's patch, except it uses the
elementtable. For the record, this is one of those times when working on this
code is fun (when you make a small change to a property and the algorithms just
handle it).

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 166726 [details] [diff] [review]
whack the parser in a friendlier way

jst, this is basically your patch, just using the element table to describe
what we want to happen with endnote.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 166726 [details] [diff] [review]
whack the parser in a friendlier way

Add a comment for why this is needed, and r+sr=bzbarsky

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked in:

Checking in src/nsElementTable.cpp;
/cvsroot/mozilla/parser/htmlparser/src/nsElementTable.cpp,v  <--  nsElementTable
.cpp
new revision: 3.163; previous revision: 3.162
done

Comment 14

[Reed Loden [:reed]]

*** Bug 317632 has been marked as a duplicate of this bug. ***