Changeset 149754 in webkit


Ignore:
Timestamp:
May 8, 2013 11:54:24 AM (11 years ago)
Author:
Darin Adler
Message:

REGRESSION(r149700): fast/css-generated-content/close-quote-negative-depth.html
https://bugs.webkit.org/show_bug.cgi?id=115776

Reviewed by Anders Carlsson.

Source/WebCore:

I changed depth to more closely match what is in the CSS3 specification.
There may be a more optimal way to make it work, but this seems the most straightforward.

  • rendering/RenderQuote.cpp:

(WebCore::RenderQuote::RenderQuote): Initialize m_depth to -1 because that depth
is consistent with the empty string that is the initial value of the text. The
real depth will be calculated when the node is attached.
(WebCore::RenderQuote::originalText): Removed the "depth - 1" logic that
used to be done for close quotes. Instead, the updateDepth function now correctly
subtracts one for the close quote itself, not just afterward. Also added an early
exit when the depth is negative; these changes together fix the bug.
(WebCore::RenderQuote::attachQuote): Added a call to updateDepth even for the render
quote head, we now need that to set the depth either to 0 or to -1.
(WebCore::RenderQuote::detachQuote): Removed code to set m_depth to 0; if we are not
resetting the text then m_depth should be left matching the text, otherwise updateDepth
might not do its job correctly if the quote is later re-attached. What matters is that
m_depth and the text are in sync.
(WebCore::RenderQuote::updateDepth): Changed updating logic in two ways. First,
compute the depth in a local variable rather than computing it in a data member
after first saving off the old value of the data member. That's clearer style.
Second, add the code to change negative depths to zero when propagating to the
next quote in the chain, which matches how the standard is written, and decrement
the depth of the close quote itself, not the quote after the close quote.

LayoutTests:

Location:
trunk
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r149749 r149754  
     12013-05-08  Darin Adler  <darin@apple.com>
     2
     3        REGRESSION(r149700): fast/css-generated-content/close-quote-negative-depth.html
     4        https://bugs.webkit.org/show_bug.cgi?id=115776
     5
     6        Reviewed by Anders Carlsson.
     7
     8        * TestExpectations: Expect success again on this test.
     9
    1102013-05-08  Eric Carlson  <eric.carlson@apple.com>
    211
  • trunk/LayoutTests/TestExpectations

    r149715 r149754  
    1111# pending CSS grammar refactoring
    1212webkit.org/b/113401 inspector/console/console-css-warnings.html [ Skip ]
    13 
    14 webkit.org/b/115776 fast/css-generated-content/close-quote-negative-depth.html [ ImageOnlyFailure ]
  • trunk/Source/WebCore/ChangeLog

    r149749 r149754  
     12013-05-08  Darin Adler  <darin@apple.com>
     2
     3        REGRESSION(r149700): fast/css-generated-content/close-quote-negative-depth.html
     4        https://bugs.webkit.org/show_bug.cgi?id=115776
     5
     6        Reviewed by Anders Carlsson.
     7
     8        I changed depth to more closely match what is in the CSS3 specification.
     9        There may be a more optimal way to make it work, but this seems the most straightforward.
     10
     11        * rendering/RenderQuote.cpp:
     12        (WebCore::RenderQuote::RenderQuote): Initialize m_depth to -1 because that depth
     13        is consistent with the empty string that is the initial value of the text. The
     14        real depth will be calculated when the node is attached.
     15        (WebCore::RenderQuote::originalText): Removed the "depth - 1" logic that
     16        used to be done for close quotes. Instead, the updateDepth function now correctly
     17        subtracts one for the close quote itself, not just afterward. Also added an early
     18        exit when the depth is negative; these changes together fix the bug.
     19        (WebCore::RenderQuote::attachQuote): Added a call to updateDepth even for the render
     20        quote head, we now need that to set the depth either to 0 or to -1.
     21        (WebCore::RenderQuote::detachQuote): Removed code to set m_depth to 0;  if we are not
     22        resetting the text then m_depth should be left matching the text, otherwise updateDepth
     23        might not do its job correctly if the quote is later re-attached. What matters is that
     24        m_depth and the text are in sync.
     25        (WebCore::RenderQuote::updateDepth): Changed updating logic in two ways. First,
     26        compute the depth in a local variable rather than computing it in a data member
     27        after first saving off the old value of the data member. That's clearer style.
     28        Second, add the code to change negative depths to zero when propagating to the
     29        next quote in the chain, which matches how the standard is written, and decrement
     30        the depth of the close quote itself, not the quote after the close quote.
     31
    1322013-05-08  Eric Carlson  <eric.carlson@apple.com>
    233
  • trunk/Source/WebCore/rendering/RenderQuote.cpp

    r149700 r149754  
    1 /**
     1/*
    22 * Copyright (C) 2011 Nokia Inc.  All rights reserved.
    33 * Copyright (C) 2012 Google Inc. All rights reserved.
     4 * Copyright (C) 2013 Apple Inc. All rights reserved.
    45 *
    56 * This library is free software; you can redistribute it and/or
     
    3334    : RenderText(node, StringImpl::empty())
    3435    , m_type(quote)
    35     , m_depth(0)
     36    , m_depth(-1)
    3637    , m_next(0)
    3738    , m_previous(0)
     
    236237PassRefPtr<StringImpl> RenderQuote::originalText() const
    237238{
     239    if (m_depth < 0)
     240        return StringImpl::empty();
    238241    switch (m_type) {
    239242    case NO_OPEN_QUOTE:
     
    241244        return StringImpl::empty();
    242245    case CLOSE_QUOTE:
    243         return quotesData()->closeQuote(m_depth - 1).impl();
     246        return quotesData()->closeQuote(m_depth).impl();
    244247    case OPEN_QUOTE:
    245248        return quotesData()->openQuote(m_depth).impl();
     
    273276        view()->setRenderQuoteHead(this);
    274277        m_attached = true;
     278        updateDepth();
    275279        return;
    276280    }
     
    325329    m_next = 0;
    326330    m_previous = 0;
    327     m_depth = 0;
    328331}
    329332
     
    331334{
    332335    ASSERT(m_attached);
    333     int oldDepth = m_depth;
    334     m_depth = 0;
     336    int depth = 0;
    335337    if (m_previous) {
    336         m_depth = m_previous->m_depth;
     338        depth = m_previous->m_depth;
     339        if (depth < 0)
     340            depth = 0;
    337341        switch (m_previous->m_type) {
    338342        case OPEN_QUOTE:
    339343        case NO_OPEN_QUOTE:
    340             m_depth++;
     344            depth++;
    341345            break;
    342346        case CLOSE_QUOTE:
    343347        case NO_CLOSE_QUOTE:
    344             if (m_depth)
    345                 m_depth--;
    346348            break;
    347349        }
    348350    }
    349     if (oldDepth != m_depth)
    350         setText(originalText());
     351    switch (m_type) {
     352    case OPEN_QUOTE:
     353    case NO_OPEN_QUOTE:
     354        break;
     355    case CLOSE_QUOTE:
     356    case NO_CLOSE_QUOTE:
     357        depth--;
     358        break;
     359    }
     360    if (m_depth == depth)
     361        return;
     362    m_depth = depth;
     363    setText(originalText());
    351364}
    352365
Note: See TracChangeset for help on using the changeset viewer.