Changeset 48764 in webkit


Ignore:
Timestamp:
Sep 25, 2009 11:55:42 AM (15 years ago)
Author:
adele@apple.com
Message:

WebCore: Fix for ​https://bugs.webkit.org/show_bug.cgi?id=29740
<rdar://problem/7168738> Gmail: After changing a foreground text color, pressing return doesn't apply background to new line

Patch by Enrica Casucci <​enrica@apple.com> on 2009-09-25
Reviewed by Darin Adler, Dan Bernstein, Adele Peterson, and others.

Change the way style is preserved when inserting a new paragraph.
The original code handled insertion at the beginning and at the end of a paragraph as special
cases. The newly created paragraph contained a set of nodes generated starting from the
computed style of the insertion node. This approach has two problems:

  1. if the insertion node has a non opaque background color and one of the parent element did have

a solid background color the new paragraph did not have the element with the solid color in the tree.

  1. in some circumstances it generated more markup than the original paragraph had (a span with bold, italic,

background color and some font attribute was being reproduced as span + bold + italic + font as separate tags.
The new approach is to recreate in the new paragraph the same hierarchy of nodes found in the
paragraph where the insertion point is.

Test: editing/inserting/insert-bg-font.html

  • editing/InsertParagraphSeparatorCommand.cpp:

(WebCore::InsertParagraphSeparatorCommand::getAncestorsInsideBlock): retrieves the list of all the ancestors
between the insert node and the outer block.
(WebCore::InsertParagraphSeparatorCommand::cloneHierarchyUnderNewBlock): uses the list of ancestors to recreate
in the new paragraph the same element hierarchy present in the starting paragraph.
(WebCore::InsertParagraphSeparatorCommand::doApply): changed the code to handle the general case of insertion
in the middle of the paragraph to use the new methods. Changed the handling of the insertion at the beginning and
at the end of the paragraph to use the new methods instead of applying the calculated style.

  • editing/InsertParagraphSeparatorCommand.h: added methods getAncestorsInsideBlock and cloneHierarchyUnderNewBlock.

LayoutTests: Updated the expected results to reflect the changes in the way the new paragraph
is created and added test case for ​https://bugs.webkit.org/show_bug.cgi?id=29740
<rdar://problem/7168738> Gmail: After changing a foreground text color, pressing return doesn't apply background to new line

Patch by Enrica Casucci <​enrica@apple.com> on 2009-09-25
Reviewed by Darin Adler, Dan Bernstein, Adele Peterson, and others.

  • editing/inserting/insert-bg-font.html: Added.
  • platform/mac/editing/inserting/insert-bg-font-expected.txt: Added.
  • platform/mac/editing/pasteboard/5478250-expected.txt:
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r48761 r48764  
     12009-09-25  Enrica Casucci  <enrica@apple.com>
     2
     3        Reviewed by Darin Adler, Dan Bernstein, Adele Peterson, and others.
     4
     5        Updated the expected results to reflect the changes in the way the new paragraph
     6        is created and added test case for https://bugs.webkit.org/show_bug.cgi?id=29740
     7        <rdar://problem/7168738> Gmail: After changing a foreground text color, pressing return doesn't apply background to new line
     8
     9        * editing/inserting/insert-bg-font.html: Added.
     10        * platform/mac/editing/inserting/insert-bg-font-expected.txt: Added.
     11        * platform/mac/editing/pasteboard/5478250-expected.txt:
     12
    1132009-09-25  Yuan Song  <song.yuan@ericsson.com>
    214

  • trunk/LayoutTests/platform/mac/editing/pasteboard/5478250-expected.txt

    r43215 r48764  
    1818              text run at (108,0) width 34: "bold."
    1919        RenderBlock {DIV} at (0,18) size 784x18
    20           RenderInline {B} at (0,0) size 393x18
     20          RenderInline {SPAN} at (0,0) size 393x18
    2121            RenderText {#text} at (0,0) size 393x18
    2222              text run at (0,0) width 393: "This text should bold and left justified with \"Some text...\"."
    23 caret: position 61 of child 0 {#text} of child 0 {B} of child 2 {DIV} of child 4 {DIV} of child 1 {BODY} of child 0 {HTML} of document
     23caret: position 61 of child 0 {#text} of child 0 {SPAN} of child 2 {DIV} of child 4 {DIV} of child 1 {BODY} of child 0 {HTML} of document

  • trunk/WebCore/ChangeLog

    r48763 r48764  
     12009-09-25  Enrica Casucci  <enrica@apple.com>
     2
     3        Reviewed by Darin Adler, Dan Bernstein, Adele Peterson, and others.
     4
     5        Fix for https://bugs.webkit.org/show_bug.cgi?id=29740
     6        <rdar://problem/7168738> Gmail: After changing a foreground text color, pressing return doesn't apply background to new line
     7
     8        Change the way style is preserved when inserting a new paragraph.
     9        The original code handled insertion at the beginning and at the end of a paragraph as special
     10        cases. The newly created paragraph contained a set of nodes generated starting from the
     11        computed style of the insertion node. This approach has two problems:
     12        1. if the insertion node has a non opaque background color and one of the parent element did have
     13        a solid background color the new paragraph did not have the element with the solid color in the tree.
     14        2. in some circumstances it generated more markup than the original paragraph had (a span with bold, italic,
     15        background color and some font attribute was being reproduced as span + bold + italic + font as separate tags.
     16        The new approach is to recreate in the new paragraph the same hierarchy of nodes found in the
     17        paragraph where the insertion point is.
     18
     19        Test: editing/inserting/insert-bg-font.html
     20
     21        * editing/InsertParagraphSeparatorCommand.cpp:
     22        (WebCore::InsertParagraphSeparatorCommand::getAncestorsInsideBlock): retrieves the list of all the ancestors
     23        between the insert node and the outer block.
     24        (WebCore::InsertParagraphSeparatorCommand::cloneHierarchyUnderNewBlock): uses the list of ancestors to recreate
     25        in the new paragraph the same element hierarchy present in the starting paragraph.
     26        (WebCore::InsertParagraphSeparatorCommand::doApply): changed the code to handle the general case of insertion
     27        in the middle of the paragraph to use the new methods. Changed the handling of the insertion at the beginning and
     28        at the end of the paragraph to use the new methods instead of applying the calculated style.
     29        * editing/InsertParagraphSeparatorCommand.h: added methods getAncestorsInsideBlock and cloneHierarchyUnderNewBlock.
     30
    1312009-09-25  Patrick Mueller  <Patrick_Mueller@us.ibm.com>
    232

  • trunk/WebCore/editing/InsertParagraphSeparatorCommand.cpp

    r48234 r48764  
    104104}
    105105
     106void InsertParagraphSeparatorCommand::getAncestorsInsideBlock(const Node* insertionNode, Element* outerBlock, Vector<Element*>& ancestors)
     107{
     108    ancestors.clear();
     109   
     110    // Build up list of ancestors elements between the insertion node and the outer block.
     111    if (insertionNode != outerBlock) {
     112        for (Element* n = insertionNode->parentElement(); n && n != outerBlock; n = n->parentElement())
     113            ancestors.append(n);
     114    }
     115}
     116
     117PassRefPtr<Element> InsertParagraphSeparatorCommand::cloneHierarchyUnderNewBlock(const Vector<Element*>& ancestors, PassRefPtr<Element> blockToInsert)
     118{
     119    // Make clones of ancestors in between the start node and the start block.
     120    RefPtr<Element> parent = blockToInsert;
     121    for (size_t i = ancestors.size(); i != 0; --i) {
     122        RefPtr<Element> child = ancestors[i - 1]->cloneElementWithoutChildren();
     123        appendNode(child, parent);
     124        parent = child.release();
     125    }
     126   
     127    return parent.release();
     128}
     129
    106130void InsertParagraphSeparatorCommand::doApply()
    107131{
    … …  
    194218        }
    195219
    196         appendBlockPlaceholder(blockToInsert);
    197         setEndingSelection(VisibleSelection(Position(blockToInsert.get(), 0), DOWNSTREAM));
    198         if (shouldApplyStyleAfterInsertion)
    199             applyStyleAfterInsertion(startBlock);
    200         return;
    201     }
     220        // Recreate the same structure in the new paragraph.
     221       
     222        Vector<Element*> ancestors;
     223        getAncestorsInsideBlock(insertionPosition.node(), startBlock, ancestors);     
     224        RefPtr<Element> parent = cloneHierarchyUnderNewBlock(ancestors, blockToInsert);
     225       
     226        appendBlockPlaceholder(parent);
     227
     228        setEndingSelection(VisibleSelection(Position(parent.get(), 0), DOWNSTREAM));
     229        return;
     230    }
     231   
    202232
    203233    //---------------------------------------------------------------------
    … …  
    218248       
    219249        insertNodeBefore(blockToInsert, refNode);
    220         appendBlockPlaceholder(blockToInsert.get());
    221         setEndingSelection(VisibleSelection(Position(blockToInsert.get(), 0), DOWNSTREAM));
    222         applyStyleAfterInsertion(startBlock);
     250
     251        // Recreate the same structure in the new paragraph.
     252
     253        Vector<Element*> ancestors;
     254        getAncestorsInsideBlock(positionAvoidingSpecialElementBoundary(insertionPosition).node(), startBlock, ancestors);
     255       
     256        appendBlockPlaceholder(cloneHierarchyUnderNewBlock(ancestors, blockToInsert));
     257       
     258        // In this case, we need to set the new ending selection.
    223259        setEndingSelection(VisibleSelection(insertionPosition, DOWNSTREAM));
    224260        return;
    … …  
    249285    // Build up list of ancestors in between the start node and the start block.
    250286    Vector<Element*> ancestors;
    251     if (insertionPosition.node() != startBlock) {
    252         for (Element* n = insertionPosition.node()->parentElement(); n && n != startBlock; n = n->parentElement())
    253             ancestors.append(n);
    254     }
     287    getAncestorsInsideBlock(insertionPosition.node(), startBlock, ancestors);
    255288
    256289    // Make sure we do not cause a rendered space to become unrendered.
    … …  
    285318    updateLayout();
    286319   
    287     // Make clones of ancestors in between the start node and the start block.
    288     RefPtr<Element> parent = blockToInsert;
    289     for (size_t i = ancestors.size(); i != 0; --i) {
    290         RefPtr<Element> child = ancestors[i - 1]->cloneElementWithoutChildren();
    291         appendNode(child, parent);
    292         parent = child.release();
    293     }
     320    // Make clones of ancestors in between the start node and the outer block.
     321    RefPtr<Element> parent = cloneHierarchyUnderNewBlock(ancestors, blockToInsert);
    294322
    295323    // If the paragraph separator was inserted at the end of a paragraph, an empty line must be

  • trunk/WebCore/editing/InsertParagraphSeparatorCommand.h

    r34536 r48764  
    4545    void calculateStyleBeforeInsertion(const Position&);
    4646    void applyStyleAfterInsertion(Node* originalEnclosingBlock);
     47    void getAncestorsInsideBlock(const Node* insertionNode, Element* outerBlock, Vector<Element*>& ancestors);
     48    PassRefPtr<Element> cloneHierarchyUnderNewBlock(const Vector<Element*>& ancestors, PassRefPtr<Element> blockToInsert);
    4749   
    4850    bool shouldUseDefaultParagraphElement(Node*) const;
Note: See TracChangeset for help on using the changeset viewer.