Changeset 62924 in webkit


Ignore:
Timestamp:
Jul 9, 2010 2:57:52 AM (14 years ago)
Author:
abarth@webkit.org
Message:

2010-07-09 Adam Barth <abarth@webkit.org>

Reviewed by Eric Seidel.

Coalesce text nodes when foster parenting
https://bugs.webkit.org/show_bug.cgi?id=41921

Yay test progression.

  • html5lib/runner-expected-html5.txt:

2010-07-09 Adam Barth <abarth@webkit.org>

Reviewed by Eric Seidel.

Coalesce text nodes when foster parenting
https://bugs.webkit.org/show_bug.cgi?id=41921

Introduces the notion of an AttachmentSite to the overall
HTMLConstructionSite. Maybe we should rename HTMLConstructionSite to
HTMLConstructionArea since we construct things all over the tree? :)

There's something wrong in the internal layering in this class, but I
can't quite see what it is. I added a FIXME for the some of the
symptoms.

  • html/HTMLConstructionSite.cpp: (WebCore::HTMLConstructionSite::attach): (WebCore::HTMLConstructionSite::attachAtSite): (WebCore::HTMLConstructionSite::insertTextNode): (WebCore::HTMLConstructionSite::findFosterSite): (WebCore::HTMLConstructionSite::fosterParent):
  • html/HTMLConstructionSite.h:
Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r62922 r62924  
     12010-07-09  Adam Barth  <abarth@webkit.org>
     2
     3        Reviewed by Eric Seidel.
     4
     5        Coalesce text nodes when foster parenting
     6        https://bugs.webkit.org/show_bug.cgi?id=41921
     7
     8        Yay test progression.
     9
     10        * html5lib/runner-expected-html5.txt:
     11
    1122010-07-08  Nikolas Zimmermann  <nzimmermann@rim.com>
    213
  • trunk/LayoutTests/html5lib/runner-expected-html5.txt

    r62914 r62924  
    66resources/tests1.dat:
    7730
    8 32
    9852
    10953
    111078
    12 79
    131180
    1412103
     
    5452|     <a>
    5553|       "Y"
    56 
    57 Test 32 of 113 in resources/tests1.dat failed. Input:
    58 <!-----><font><div>hello<table>excite!<b>me!<th><i>please!</tr><!--X-->
    59 Got:
    60 | <!-- - -->
    61 | <html>
    62 |   <head>
    63 |   <body>
    64 |     <font>
    65 |       <div>
    66 |         "hello"
    67 |         "excite!"
    68 |         <b>
    69 |           "me!"
    70 |         <table>
    71 |           <tbody>
    72 |             <tr>
    73 |               <th>
    74 |                 <i>
    75 |                   "please!"
    76 |             <!-- X -->
    77 Expected:
    78 | <!-- - -->
    79 | <html>
    80 |   <head>
    81 |   <body>
    82 |     <font>
    83 |       <div>
    84 |         "helloexcite!"
    85 |         <b>
    86 |           "me!"
    87 |         <table>
    88 |           <tbody>
    89 |             <tr>
    90 |               <th>
    91 |                 <i>
    92 |                   "please!"
    93 |             <!-- X -->
    9454
    9555Test 52 of 113 in resources/tests1.dat failed. Input:
     
    180140|       "aoe"
    181141
    182 Test 79 of 113 in resources/tests1.dat failed. Input:
    183 <a href="blah">aba<table><tr><td><a href="foo">br</td></tr>x</table>aoe
    184 Got:
    185 | <html>
    186 |   <head>
    187 |   <body>
    188 |     <a>
    189 |       href="blah"
    190 |       "aba"
    191 |       "x"
    192 |       <table>
    193 |         <tbody>
    194 |           <tr>
    195 |             <td>
    196 |               <a>
    197 |                 href="foo"
    198 |                 "br"
    199 |       "aoe"
    200 Expected:
    201 | <html>
    202 |   <head>
    203 |   <body>
    204 |     <a>
    205 |       href="blah"
    206 |       "abax"
    207 |       <table>
    208 |         <tbody>
    209 |           <tr>
    210 |             <td>
    211 |               <a>
    212 |                 href="foo"
    213 |                 "br"
    214 |       "aoe"
    215 
    216142Test 80 of 113 in resources/tests1.dat failed. Input:
    217143<table><a href="blah">aba<tr><td><a href="foo">br</td></tr>x</table>aoe
     
    45638224
    45738327
    458 28
    459 29
    46038430
    461385
     
    497421|     <b>
    498422|       "ccc"
    499 
    500 Test 28 of 30 in resources/tests7.dat failed. Input:
    501 A<table><tr> B</tr> B</table>
    502 Got:
    503 | <html>
    504 |   <head>
    505 |   <body>
    506 |     "A"
    507 |     " B"
    508 |     " B"
    509 |     <table>
    510 |       <tbody>
    511 |         <tr>
    512 Expected:
    513 | <html>
    514 |   <head>
    515 |   <body>
    516 |     "A B B"
    517 |     <table>
    518 |       <tbody>
    519 |         <tr>
    520 
    521 Test 29 of 30 in resources/tests7.dat failed. Input:
    522 A<table><tr> B</tr> </em>C</table>
    523 Got:
    524 | <html>
    525 |   <head>
    526 |   <body>
    527 |     "A"
    528 |     " B"
    529 |     <table>
    530 |       <tbody>
    531 |         <tr>
    532 |         " C"
    533 Expected:
    534 | <html>
    535 |   <head>
    536 |   <body>
    537 |     "A BC"
    538 |     <table>
    539 |       <tbody>
    540 |         <tr>
    541 |         " "
    542423
    543424Test 30 of 30 in resources/tests7.dat failed. Input:
     
    561442|     <select>
    562443|     <keygen>
    563 resources/tests8.dat:
    564 6
    565 
    566 Test 6 of 9 in resources/tests8.dat failed. Input:
    567 x<table>x
    568 Got:
    569 | <html>
    570 |   <head>
    571 |   <body>
    572 |     "x"
    573 |     "x"
    574 |     <table>
    575 Expected:
    576 | <html>
    577 |   <head>
    578 |   <body>
    579 |     "xx"
    580 |     <table>
     444resources/tests8.dat: PASS
     445
    581446resources/tests9.dat:
    5824471
  • trunk/WebCore/ChangeLog

    r62922 r62924  
     12010-07-09  Adam Barth  <abarth@webkit.org>
     2
     3        Reviewed by Eric Seidel.
     4
     5        Coalesce text nodes when foster parenting
     6        https://bugs.webkit.org/show_bug.cgi?id=41921
     7
     8        Introduces the notion of an AttachmentSite to the overall
     9        HTMLConstructionSite.  Maybe we should rename HTMLConstructionSite to
     10        HTMLConstructionArea since we construct things all over the tree?  :)
     11
     12        There's something wrong in the internal layering in this class, but I
     13        can't quite see what it is.  I added a FIXME for the some of the
     14        symptoms.
     15
     16        * html/HTMLConstructionSite.cpp:
     17        (WebCore::HTMLConstructionSite::attach):
     18        (WebCore::HTMLConstructionSite::attachAtSite):
     19        (WebCore::HTMLConstructionSite::insertTextNode):
     20        (WebCore::HTMLConstructionSite::findFosterSite):
     21        (WebCore::HTMLConstructionSite::fosterParent):
     22        * html/HTMLConstructionSite.h:
     23
    1242010-07-08  Nikolas Zimmermann  <nzimmermann@rim.com>
    225
  • trunk/WebCore/html/HTMLConstructionSite.cpp

    r62916 r62924  
    7979    RefPtr<ChildType> child = prpChild;
    8080
     81    // FIXME: It's confusing that HTMLConstructionSite::attach does the magic
     82    // redirection to the foster parent but HTMLConstructionSite::attachAtSite
     83    // doesn't.  It feels like we're missing a concept somehow.
    8184    if (m_redirectAttachToFosterParent) {
    8285        fosterParent(child.get());
     
    9497}
    9598
     99void HTMLConstructionSite::attachAtSite(const AttachmentSite& site, PassRefPtr<Node> child)
     100{
     101    if (site.nextChild) {
     102        // FIXME: We need an insertElement which does not send mutation events.
     103        ExceptionCode ec = 0;
     104        site.parent->insertBefore(child, site.nextChild, ec);
     105        // FIXME: Do we need to call attach()?
     106        ASSERT(!ec);
     107        return;
     108    }
     109    site.parent->parserAddChild(child);
     110    // FIXME: Do we need to call attach()?
     111}
     112
    96113HTMLConstructionSite::HTMLConstructionSite(Document* document, FragmentScriptingPermission scriptingPermission)
    97114    : m_document(document)
     
    232249void HTMLConstructionSite::insertTextNode(const String& characters)
    233250{
    234     if (Node* lastChild = currentElement()->lastChild()) {
    235         if (lastChild->isTextNode()) {
    236             // FIXME: We're only supposed to append to this text node if it
    237             // was the last text node inserted by the parser.
    238             CharacterData* textNode = static_cast<CharacterData*>(lastChild);
    239             textNode->parserAppendData(characters);
    240             return;
    241         }
    242     }
    243     attach(currentElement(), Text::create(m_document, characters));
     251    AttachmentSite site;
     252    site.parent = currentElement();
     253    site.nextChild = 0;
     254    if (m_redirectAttachToFosterParent)
     255        findFosterSite(site);
     256
     257    Node* previousChild = site.nextChild ? site.nextChild->previousSibling() : site.parent->lastChild();
     258    if (previousChild && previousChild->isTextNode()) {
     259        // FIXME: We're only supposed to append to this text node if it
     260        // was the last text node inserted by the parser.
     261        CharacterData* textNode = static_cast<CharacterData*>(previousChild);
     262        textNode->parserAppendData(characters);
     263        return;
     264    }
     265
     266    attachAtSite(site, Text::create(m_document, characters));
    244267}
    245268
     
    305328}
    306329
    307 void HTMLConstructionSite::fosterParent(Node* node)
    308 {
    309     Element* fosterParentElement = 0;
     330void HTMLConstructionSite::findFosterSite(AttachmentSite& site)
     331{
    310332    HTMLElementStack::ElementRecord* lastTableElementRecord = m_openElements.topmost(tableTag.localName());
    311333    if (lastTableElementRecord) {
    312334        Element* lastTableElement = lastTableElementRecord->element();
    313         if (lastTableElement->parent()) {
    314             // FIXME: We need an insertHTMLElement which does not send mutation events.
    315             ExceptionCode ec = 0;
    316             lastTableElement->parent()->insertBefore(node, lastTableElement, ec);
    317             // FIXME: Do we need to call attach()?
    318             ASSERT(!ec);
     335        if (Node* parent = lastTableElement->parent()) {
     336            site.parent = parent;
     337            site.nextChild = lastTableElement;
    319338            return;
    320339        }
    321         fosterParentElement = lastTableElementRecord->next()->element();
    322     } else {
    323         // Fragment case
    324         fosterParentElement = m_openElements.bottom(); // <html> element
    325     }
    326 
    327     fosterParentElement->parserAddChild(node);
    328     // FIXME: Do we need to call attach()?
    329 }
    330 
    331 }
     340        site.parent = lastTableElementRecord->next()->element();
     341        site.nextChild = 0;
     342        return;
     343    }
     344    // Fragment case
     345    site.parent = m_openElements.bottom(); // <html> element
     346    site.nextChild = 0;
     347}
     348
     349void HTMLConstructionSite::fosterParent(Node* node)
     350{
     351    AttachmentSite site;
     352    findFosterSite(site);
     353    attachAtSite(site, node);
     354}
     355
     356}
  • trunk/WebCore/html/HTMLConstructionSite.h

    r62916 r62924  
    109109    friend class RedirectToFosterParentGuard;
    110110
     111    struct AttachmentSite {
     112        Node* parent;
     113        Node* nextChild;
     114    };
     115
    111116    template<typename ChildType>
    112     PassRefPtr<ChildType> attach(Node* parent, PassRefPtr<ChildType> prpChild);
     117    PassRefPtr<ChildType> attach(Node* parent, PassRefPtr<ChildType> child);
     118
     119    void attachAtSite(const AttachmentSite&, PassRefPtr<Node> child);
     120    void findFosterSite(AttachmentSite&);
    113121
    114122    PassRefPtr<Element> createElement(AtomicHTMLToken&, const AtomicString& namespaceURI);
Note: See TracChangeset for help on using the changeset viewer.