Title: [149754] trunk
Revision
149754
Author
[email protected]
Date
2013-05-08 11:54:24 -0700 (Wed, 08 May 2013)

Log 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:

* TestExpectations: Expect success again on this test.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (149753 => 149754)


--- trunk/LayoutTests/ChangeLog	2013-05-08 18:52:01 UTC (rev 149753)
+++ trunk/LayoutTests/ChangeLog	2013-05-08 18:54:24 UTC (rev 149754)
@@ -1,3 +1,12 @@
+2013-05-08  Darin Adler  <[email protected]>
+
+        REGRESSION(r149700): fast/css-generated-content/close-quote-negative-depth.html
+        https://bugs.webkit.org/show_bug.cgi?id=115776
+
+        Reviewed by Anders Carlsson.
+
+        * TestExpectations: Expect success again on this test.
+
 2013-05-08  Eric Carlson  <[email protected]>
 
         Prevent crash when track is deleted during video element deletion.

Modified: trunk/LayoutTests/TestExpectations (149753 => 149754)


--- trunk/LayoutTests/TestExpectations	2013-05-08 18:52:01 UTC (rev 149753)
+++ trunk/LayoutTests/TestExpectations	2013-05-08 18:54:24 UTC (rev 149754)
@@ -10,5 +10,3 @@
 
 # pending CSS grammar refactoring
 webkit.org/b/113401 inspector/console/console-css-warnings.html [ Skip ]
-
-webkit.org/b/115776 fast/css-generated-content/close-quote-negative-depth.html [ ImageOnlyFailure ]

Modified: trunk/Source/WebCore/ChangeLog (149753 => 149754)


--- trunk/Source/WebCore/ChangeLog	2013-05-08 18:52:01 UTC (rev 149753)
+++ trunk/Source/WebCore/ChangeLog	2013-05-08 18:54:24 UTC (rev 149754)
@@ -1,3 +1,34 @@
+2013-05-08  Darin Adler  <[email protected]>
+
+        REGRESSION(r149700): fast/css-generated-content/close-quote-negative-depth.html
+        https://bugs.webkit.org/show_bug.cgi?id=115776
+
+        Reviewed by Anders Carlsson.
+
+        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.
+
 2013-05-08  Eric Carlson  <[email protected]>
 
         Prevent crash when track is deleted during video element deletion.

Modified: trunk/Source/WebCore/rendering/RenderQuote.cpp (149753 => 149754)


--- trunk/Source/WebCore/rendering/RenderQuote.cpp	2013-05-08 18:52:01 UTC (rev 149753)
+++ trunk/Source/WebCore/rendering/RenderQuote.cpp	2013-05-08 18:54:24 UTC (rev 149754)
@@ -1,6 +1,7 @@
-/**
+/*
  * Copyright (C) 2011 Nokia Inc.  All rights reserved.
  * Copyright (C) 2012 Google Inc. All rights reserved.
+ * Copyright (C) 2013 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -32,7 +33,7 @@
 RenderQuote::RenderQuote(Document* node, QuoteType quote)
     : RenderText(node, StringImpl::empty())
     , m_type(quote)
-    , m_depth(0)
+    , m_depth(-1)
     , m_next(0)
     , m_previous(0)
     , m_attached(false)
@@ -235,12 +236,14 @@
 
 PassRefPtr<StringImpl> RenderQuote::originalText() const
 {
+    if (m_depth < 0)
+        return StringImpl::empty();
     switch (m_type) {
     case NO_OPEN_QUOTE:
     case NO_CLOSE_QUOTE:
         return StringImpl::empty();
     case CLOSE_QUOTE:
-        return quotesData()->closeQuote(m_depth - 1).impl();
+        return quotesData()->closeQuote(m_depth).impl();
     case OPEN_QUOTE:
         return quotesData()->openQuote(m_depth).impl();
     }
@@ -272,6 +275,7 @@
     if (!view()->renderQuoteHead()) {
         view()->setRenderQuoteHead(this);
         m_attached = true;
+        updateDepth();
         return;
     }
 
@@ -324,30 +328,39 @@
     m_attached = false;
     m_next = 0;
     m_previous = 0;
-    m_depth = 0;
 }
 
 void RenderQuote::updateDepth()
 {
     ASSERT(m_attached);
-    int oldDepth = m_depth;
-    m_depth = 0;
+    int depth = 0;
     if (m_previous) {
-        m_depth = m_previous->m_depth;
+        depth = m_previous->m_depth;
+        if (depth < 0)
+            depth = 0;
         switch (m_previous->m_type) {
         case OPEN_QUOTE:
         case NO_OPEN_QUOTE:
-            m_depth++;
+            depth++;
             break;
         case CLOSE_QUOTE:
         case NO_CLOSE_QUOTE:
-            if (m_depth)
-                m_depth--;
             break;
         }
     }
-    if (oldDepth != m_depth)
-        setText(originalText());
+    switch (m_type) {
+    case OPEN_QUOTE:
+    case NO_OPEN_QUOTE:
+        break;
+    case CLOSE_QUOTE:
+    case NO_CLOSE_QUOTE:
+        depth--;
+        break;
+    }
+    if (m_depth == depth)
+        return;
+    m_depth = depth;
+    setText(originalText());
 }
 
 } // namespace WebCore
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to