Changeset 63386 in webkit


Ignore:
Timestamp:
Jul 14, 2010 6:06:11 PM (14 years ago)
Author:
eric@webkit.org
Message:

2010-07-14 Eric Seidel <​eric@webkit.org>

Reviewed by Adam Barth.

HTMLTreeBuilder foster parents when it should not
​https://bugs.webkit.org/show_bug.cgi?id=42235

  • html5lib/resources/adoption01.dat:
  • html5lib/runner-expected-html5.txt:
  • html5lib/runner-expected.txt:

2010-07-14 Eric Seidel <​eric@webkit.org>

Reviewed by Adam Barth.

HTMLTreeBuilder foster parents when it should not
​https://bugs.webkit.org/show_bug.cgi?id=42235

Regarding foster parenting of nodes inside tables:
"Process the token using the rules for the "in body" insertion mode,
except that if the current node is a table, tbody, tfoot, thead, or
tr element, then, whenever a node would be inserted into the current
node, it must instead be foster parented."

We were forgetting the "when the current node is" part of that check
and always foster parenting, even if we had just inserted another
element (which would have just changed the current node).

This was covered by multiple tests in html5lib/runner.html
but I wrote a reduction (one which I included) as it makes it
easier to see what's going on.

  • html/HTMLConstructionSite.cpp: (WebCore::HTMLNames::causesFosterParenting): (WebCore::HTMLConstructionSite::attach): (WebCore::HTMLConstructionSite::insertHTMLHtmlElement): (WebCore::HTMLConstructionSite::insertHTMLHeadElement): (WebCore::HTMLConstructionSite::insertHTMLBodyElement): (WebCore::HTMLConstructionSite::insertTextNode): (WebCore::HTMLConstructionSite::shouldFosterParent):
  • html/HTMLConstructionSite.h:
  • html/HTMLTreeBuilder.cpp: (WebCore::HTMLTreeBuilder::callTheAdoptionAgency):
Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r63381 r63386  
     12010-07-14  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        HTMLTreeBuilder foster parents when it should not
     6        https://bugs.webkit.org/show_bug.cgi?id=42235
     7
     8        * html5lib/resources/adoption01.dat:
     9        * html5lib/runner-expected-html5.txt:
     10        * html5lib/runner-expected.txt:
     11
    1122010-07-14  Alexey Proskuryakov  <ap@apple.com>
    213

  • trunk/LayoutTests/html5lib/resources/adoption01.dat

    r63338 r63386  
    165165|       id="A"
    166166|       "TEXT"
     167
     168#data
     169<table><a>1<td>2</td>3</table>
     170#errors
     171#document
     172| <html>
     173|   <head>
     174|   <body>
     175|     <a>
     176|       "1"
     177|     <a>
     178|       "3"
     179|     <table>
     180|       <tbody>
     181|         <tr>
     182|           <td>
     183|             "2"

  • trunk/LayoutTests/html5lib/runner-expected-html5.txt

    r63338 r63386  
    77CONSOLE MESSAGE: line 2: FOO<span>BAR</span>BAZ
    88Content-Type: text/plain
    9 resources/tests1.dat:
    10 78
    11 80
    12 
    13 Test 78 of 113 in resources/tests1.dat failed. Input:
    14 <a href="blah">aba<table><a href="foo">br<tr><td></td></tr>x</table>aoe
    15 Got:
    16 | <html>
    17 |   <head>
    18 |   <body>
    19 |     <a>
    20 |       href="blah"
    21 |       "aba"
    22 |       <a>
    23 |         href="foo"
    24 |         "br"
    25 |       <a>
    26 |         href="foo"
    27 |       "x"
    28 |       <table>
    29 |         <tbody>
    30 |           <tr>
    31 |             <td>
    32 |     <a>
    33 |       href="foo"
    34 |       "aoe"
    35 Expected:
    36 | <html>
    37 |   <head>
    38 |   <body>
    39 |     <a>
    40 |       href="blah"
    41 |       "aba"
    42 |       <a>
    43 |         href="foo"
    44 |         "br"
    45 |       <a>
    46 |         href="foo"
    47 |         "x"
    48 |       <table>
    49 |         <tbody>
    50 |           <tr>
    51 |             <td>
    52 |     <a>
    53 |       href="foo"
    54 |       "aoe"
    55 
    56 Test 80 of 113 in resources/tests1.dat failed. Input:
    57 <table><a href="blah">aba<tr><td><a href="foo">br</td></tr>x</table>aoe
    58 Got:
    59 | <html>
    60 |   <head>
    61 |   <body>
    62 |     <a>
    63 |       href="blah"
    64 |       "aba"
    65 |     <a>
    66 |       href="blah"
    67 |     "x"
    68 |     <table>
    69 |       <tbody>
    70 |         <tr>
    71 |           <td>
    72 |             <a>
    73 |               href="foo"
    74 |               "br"
    75 |     <a>
    76 |       href="blah"
    77 |       "aoe"
    78 Expected:
    79 | <html>
    80 |   <head>
    81 |   <body>
    82 |     <a>
    83 |       href="blah"
    84 |       "aba"
    85 |     <a>
    86 |       href="blah"
    87 |       "x"
    88 |     <table>
    89 |       <tbody>
    90 |         <tr>
    91 |           <td>
    92 |             <a>
    93 |               href="foo"
    94 |               "br"
    95 |     <a>
    96 |       href="blah"
    97 |       "aoe"
     9resources/tests1.dat: PASS
     10
    9811resources/tests2.dat: PASS
    9912
    … …  
    207120resources/tests7.dat:
    20812124
    209 27
    21012230
    211123
    … …  
    218130| <body>
    219131|   "X"
    220 
    221 Test 27 of 30 in resources/tests7.dat failed. Input:
    222 <table><b><tr><td>aaa</td></tr>bbb</table>ccc
    223 Got:
    224 | <html>
    225 |   <head>
    226 |   <body>
    227 |     <b>
    228 |     <b>
    229 |     "bbb"
    230 |     <table>
    231 |       <tbody>
    232 |         <tr>
    233 |           <td>
    234 |             "aaa"
    235 |     <b>
    236 |       "ccc"
    237 Expected:
    238 | <html>
    239 |   <head>
    240 |   <body>
    241 |     <b>
    242 |     <b>
    243 |       "bbb"
    244 |     <table>
    245 |       <tbody>
    246 |         <tr>
    247 |           <td>
    248 |             "aaa"
    249 |     <b>
    250 |       "ccc"
    251132
    252133Test 30 of 30 in resources/tests7.dat failed. Input:
    … …  
    32921011
    330211
    331 Test 3 of 11 in resources/adoption01.dat failed. Input:
     212Test 3 of 12 in resources/adoption01.dat failed. Input:
    332213<a>1<button>2</a>3</button>
    333214Got:
    … …  
    349230|     "3"
    350231
    351 Test 11 of 11 in resources/adoption01.dat failed. Input:
     232Test 11 of 12 in resources/adoption01.dat failed. Input:
    352233<p><b id="A"><script>document.getElementById("A").id = "B"</script></p>TEXT</b>
    353234Got:

  • trunk/LayoutTests/html5lib/runner-expected.txt

    r63338 r63386  
    500550059
    5006500611
    5007 
    5008 Test 1 of 11 in resources/adoption01.dat failed. Input:
     500712
     5008
     5009Test 1 of 12 in resources/adoption01.dat failed. Input:
    50095010<a><p></a></p>
    50105011Got:
    … …  
    50225023|       <a>
    50235024
    5024 Test 6 of 11 in resources/adoption01.dat failed. Input:
     5025Test 6 of 12 in resources/adoption01.dat failed. Input:
    50255026<table><a>1<p>2</a>3</p>
    50265027Got:
    … …  
    50475048|     <table>
    50485049
    5049 Test 7 of 11 in resources/adoption01.dat failed. Input:
     5050Test 7 of 12 in resources/adoption01.dat failed. Input:
    50505051<b><b><a><p></a>
    50515052Got:
    … …  
    50675068|           <a>
    50685069
    5069 Test 8 of 11 in resources/adoption01.dat failed. Input:
     5070Test 8 of 12 in resources/adoption01.dat failed. Input:
    50705071<b><a><b><p></a>
    50715072Got:
    … …  
    50895090|           <a>
    50905091
    5091 Test 9 of 11 in resources/adoption01.dat failed. Input:
     5092Test 9 of 12 in resources/adoption01.dat failed. Input:
    50925093<a><b><b><p></a>
    50935094Got:
    … …  
    51135114|           <a>
    51145115
    5115 Test 11 of 11 in resources/adoption01.dat failed. Input:
     5116Test 11 of 12 in resources/adoption01.dat failed. Input:
    51165117<p><b id="A"><script>document.getElementById("A").id = "B"</script></p>TEXT</b>
    51175118Got:
    … …  
    51395140|       id="A"
    51405141|       "TEXT"
     5142
     5143Test 12 of 12 in resources/adoption01.dat failed. Input:
     5144<table><a>1<td>2</td>3</table>
     5145Got:
     5146| <html>
     5147|   <head>
     5148|   <body>
     5149|     <a>
     5150|       "1"
     5151|     "3"
     5152|     <table>
     5153|       <tbody>
     5154|         <tr>
     5155|           <td>
     5156|             "2"
     5157Expected:
     5158| <html>
     5159|   <head>
     5160|   <body>
     5161|     <a>
     5162|       "1"
     5163|     <a>
     5164|       "3"
     5165|     <table>
     5166|       <tbody>
     5167|         <tr>
     5168|           <td>
     5169|             "2"
    51415170resources/inbody01.dat: PASS
    51425171

  • trunk/WebCore/ChangeLog

    r63384 r63386  
     12010-07-14  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        HTMLTreeBuilder foster parents when it should not
     6        https://bugs.webkit.org/show_bug.cgi?id=42235
     7
     8        Regarding foster parenting of nodes inside tables:
     9        "Process the token using the rules for the "in body" insertion mode,
     10        except that if the current node is a table, tbody, tfoot, thead, or
     11        tr element, then, whenever a node would be inserted into the current
     12        node, it must instead be foster parented."
     13
     14        We were forgetting the "when the current node is" part of that check
     15        and always foster parenting, even if we had just inserted another
     16        element (which would have just changed the current node).
     17
     18        This was covered by multiple tests in html5lib/runner.html
     19        but I wrote a reduction (one which I included) as it makes it
     20        easier to see what's going on.
     21
     22        * html/HTMLConstructionSite.cpp:
     23        (WebCore::HTMLNames::causesFosterParenting):
     24        (WebCore::HTMLConstructionSite::attach):
     25        (WebCore::HTMLConstructionSite::insertHTMLHtmlElement):
     26        (WebCore::HTMLConstructionSite::insertHTMLHeadElement):
     27        (WebCore::HTMLConstructionSite::insertHTMLBodyElement):
     28        (WebCore::HTMLConstructionSite::insertTextNode):
     29        (WebCore::HTMLConstructionSite::shouldFosterParent):
     30        * html/HTMLConstructionSite.h:
     31        * html/HTMLTreeBuilder.cpp:
     32        (WebCore::HTMLTreeBuilder::callTheAdoptionAgency):
     33
    1342010-07-14  Brady Eidson  <beidson@apple.com>
    235

  • trunk/WebCore/html/HTMLConstructionSite.cpp

    r63338 r63386  
    7171}
    7272
     73bool causesFosterParenting(const QualifiedName& tagName)
     74{
     75    return tagName == tableTag
     76        || tagName == tbodyTag
     77        || tagName == tfootTag
     78        || tagName == theadTag
     79        || tagName == trTag;
     80}
     81
    7382} // namespace
    7483
    … …  
    8190    // redirection to the foster parent but HTMLConstructionSite::attachAtSite
    8291    // doesn't.  It feels like we're missing a concept somehow.
    83     if (m_redirectAttachToFosterParent) {
     92    if (shouldFosterParent()) {
    8493        fosterParent(child.get());
    8594        ASSERT(child->attached());
    … …  
    201210void HTMLConstructionSite::insertHTMLHtmlElement(AtomicHTMLToken& token)
    202211{
    203     ASSERT(!m_redirectAttachToFosterParent);
     212    ASSERT(!shouldFosterParent());
    204213    m_openElements.pushHTMLHtmlElement(attachToCurrent(createHTMLElement(token)));
    205214}
    … …  
    207216void HTMLConstructionSite::insertHTMLHeadElement(AtomicHTMLToken& token)
    208217{
    209     ASSERT(!m_redirectAttachToFosterParent);
     218    ASSERT(!shouldFosterParent());
    210219    m_head = attachToCurrent(createHTMLElement(token));
    211220    m_openElements.pushHTMLHeadElement(m_head);
    … …  
    214223void HTMLConstructionSite::insertHTMLBodyElement(AtomicHTMLToken& token)
    215224{
    216     ASSERT(!m_redirectAttachToFosterParent);
     225    ASSERT(!shouldFosterParent());
    217226    m_openElements.pushHTMLBodyElement(attachToCurrent(createHTMLElement(token)));
    218227}
    … …  
    262271    site.parent = currentElement();
    263272    site.nextChild = 0;
    264     if (m_redirectAttachToFosterParent)
     273    if (shouldFosterParent())
    265274        findFosterSite(site);
    266275
    … …  
    398407}
    399408
     409bool HTMLConstructionSite::shouldFosterParent() const
     410{
     411    return m_redirectAttachToFosterParent
     412        && causesFosterParenting(currentElement()->tagQName());
     413}
     414
    400415void HTMLConstructionSite::fosterParent(Node* node)
    401416{

  • trunk/WebCore/html/HTMLConstructionSite.h

    r63338 r63386  
    6767    PassRefPtr<Element> createHTMLElementFromElementRecord(HTMLElementStack::ElementRecord*);
    6868
     69    bool shouldFosterParent() const;
    6970    void fosterParent(Node*);
    7071

  • trunk/WebCore/html/HTMLTreeBuilder.cpp

    r63374 r63386  
    16961696        // 7
    16971697        const AtomicString& commonAncestorTag = commonAncestor->localName();
     1698        // FIXME: If this moves to HTMLConstructionSite, this check should use
     1699        // causesFosterParenting(tagName) instead.
    16981700        if (commonAncestorTag == tableTag
    16991701            || commonAncestorTag == trTag
Note: See TracChangeset for help on using the changeset viewer.