Bug 55229

Summary: REGRESSION (r66062) - inputElement.innerText = "" throws
Product: WebKit Reporter: Brady Eidson <beidson>
Component: HTML DOMAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: abarth, eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Description Flags
Test derived from
Better test with explanatory text none

Description Brady Eidson 2011-02-25 10:11:06 PST
REGRESSION (r66062) - set(Inner/Outer)Text on many types of elements throws an exception, and they didn't before.

Link to the change:
Link to the original bug:

ChangeLog says "this is just moving code and should have no functional change."

But by replacing the usage of "HTMLElement::endTagRequirement()" with "HTMLElement::ieForbidsInsertHTML()" in 3 key places in HTMLElement, it had functional change.

Before this change, HTMLElement::deprecatedCreateContextualFragment would only fail to return a fragment for <wbr> elements.  Now it fails for ~20 tag names listed in HTMLElement::ieForbidsInsertHTML()

Before this change, HTMLElement::setInnerText and HTMLElement::setOuterText only threw exceptions for 10 tag names they knew about, and <wbr>.
Now they throw exceptions for the 10 tag names they knew about, and the ~20 listed in HTMLElement::ieForbidsInsertHTML()

This broke compatibility with other browsers like Firefox and Opera.  It also *broke* compatibility with IE while purporting to match IE's rules, as noted in the FIXME comment in ieForbidsInsertHTML()

Test case coming soon...
Comment 1 Brady Eidson 2011-02-25 11:27:35 PST
Created attachment 83852 [details]
Test derived from

This is a test derived from a real world affected site my.schedulemaster.com

Sadly the real site requires logins and no pilot would be likely to hand theirs out (I sure am not).

But let's just say that a lot of pilots really like WebKit based browsers.  :)
Comment 2 Brady Eidson 2011-02-25 11:29:06 PST
Created attachment 83853 [details]
Better test with explanatory text
Comment 3 Brady Eidson 2011-02-25 11:32:42 PST
Comment 4 Eric Seidel 2011-02-26 16:24:39 PST
innerText isn't spec'd, but the change in question was not supposed to change behavior.  Will investigate what changed.
Comment 5 Eric Seidel 2011-02-26 16:28:12 PST
Firefox doesn't support innerText, so the testcase is misleading.  I suspect the site in question follows a different codepath for firefox.
Comment 6 Eric Seidel 2011-02-26 16:29:34 PST
Has this shipped in a Safari?  I'm testing with Version 5.0.3 (6533.19.4) and I see the same exception as top of tree.
Comment 7 Brady Eidson 2011-02-27 10:21:33 PST
(In reply to comment #6)
> Has this shipped in a Safari?  I'm testing with Version 5.0.3 (6533.19.4) and I see the same exception as top of tree.

Hmmmm.  Admittedly and amusingly I'd never tried with Safari 5.0.3, I only tried with ToT and the patch partially rolled out.  Maybe "partially" rolled out invented the state I was expecting to see.

Will explore more when I'm around code on Monday.