Changeset 116694 in webkit

Timestamp:

May 10, 2012 3:11:57 PM (12 years ago)

Author:

eric@webkit.org

Message:

Make IFRAME_SEAMLESS child documents inherit styles from their parent iframe element
​https://bugs.webkit.org/show_bug.cgi?id=85940

Reviewed by Ojan Vafai.

Source/WebCore:

The HTML5 <iframe seamless> spec says:
In a CSS-supporting user agent: the user agent must, for the purpose of CSS property
inheritance only, treat the root element of the active document of the iframe
element's nested browsing context as being a child of the iframe element.
(Thus inherited properties on the root element of the document in the
iframe will inherit the computed values of those properties on the iframe
element instead of taking their initial values.)

Initially I implemented this support to the letter of the spec. However, doing so I learned
that WebKit has a RenderStyle for the Document Node, not just the root element of the document.
In this RenderStyle on the Document, we add a bunch of per-document styles from settings
including designMode.

This change makes StyleResolver::styleForDocument inherit style from the parent iframe's
style, before applying any of these per-document styles. This may or may not be correct
depending on what behavior we want for rtl-ordering, page-zoom, locale, design mode, etc.
For now, we continue to treat the iframe's document as independent in these regards, and
the settings on that document override those inherited from the iframe.

Also, intially when making this work, I added redirects in recalcStyle and scheduleStyleRecalc
from the child document to the parent document in the case of seamless (since the parent
document effectively manages the style resolve and layout of the child in seamless mode).
However, I was not able to find a test which depended on this code change, so in this final patch
I have removed both of these modifications and replaced them with FIXMEs. Based on discussions
with Ojan and James Robinson, I believe both of those changes may eventually be wanted.

This change basically does 3 things:

1. Makes StyleResolver::styleForDocument inherit from the parent iframe.
2. Makes any recalcStyle calls on the iframe propogate down into the child document (HTMLIFrameElement::didRecalcStyle).
3. Makes Document::recalcStyle aware of the fact that the Document's style *can* change

for reasons other than recalcStyle(Force).

I'm open to more testing suggestions, if reviewers have settings on the Document's style
that you want to make sure we inherit from the parent iframe, or don't inherit, etc.
I view this as a complete solution to this aspect of the current <iframe seamless> spec,
but likely not the last code we will write for this aspect of the seamless feature. :)

Tested by fast/frames/seamlesss/seamless-css-cascade.html and seamless-designMode.html

• css/StyleResolver.cpp:

(WebCore::StyleResolver::collectMatchingRulesForList):

• dom/Document.cpp:

(WebCore::Document::scheduleStyleRecalc):
(WebCore::Document::recalcStyle):

• html/HTMLIFrameElement.cpp:

(WebCore::HTMLIFrameElement::HTMLIFrameElement):
(WebCore::HTMLIFrameElement::didRecalcStyle):
(WebCore):

• html/HTMLIFrameElement.h:

(HTMLIFrameElement):

LayoutTests:

This single pass is deceptive. seamless-designMode exists
to make sure that we do not regress application of Document-level
styles in the child document.

• fast/frames/seamless/seamless-css-cascade-expected.txt:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/frames/seamless/seamless-css-cascade-expected.txt (modified) (1 diff)
• LayoutTests/fast/frames/seamless/seamless-inherited-document-style-expected.txt (added)
• LayoutTests/fast/frames/seamless/seamless-inherited-document-style.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/StyleResolver.cpp (modified) (2 diffs)
• Source/WebCore/dom/Document.cpp (modified) (3 diffs)
• Source/WebCore/html/HTMLIFrameElement.cpp (modified) (2 diffs)
• Source/WebCore/html/HTMLIFrameElement.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-05-10  Eric Seidel  <eric@webkit.org>
+
+        Make IFRAME_SEAMLESS child documents inherit styles from their parent iframe element
+        https://bugs.webkit.org/show_bug.cgi?id=85940
+
+        Reviewed by Ojan Vafai.
+
+        This single pass is deceptive.  seamless-designMode exists
+        to make sure that we do not regress application of Document-level
+        styles in the child document.
+
+        * fast/frames/seamless/seamless-css-cascade-expected.txt:
+
 2012-05-10  Julien Chaffraix  <jchaffraix@webkit.org>
 

trunk/LayoutTests/fast/frames/seamless/seamless-css-cascade-expected.txt

@@ -3 +3 @@
 PASS window.getComputedStyle(two).color is "rgb(128, 0, 128)"
 PASS window.getComputedStyle(three).color is "rgb(255, 255, 255)"
-FAIL window.getComputedStyle(rootElement).color should be rgb(255, 165, 0). Was rgb(0, 0, 0).
-FAIL window.getComputedStyle(rootElement).color should be rgb(1, 2, 3). Was rgb(0, 0, 0).
+PASS window.getComputedStyle(rootElement).color is "rgb(255, 165, 0)"
+PASS window.getComputedStyle(rootElement).color is "rgb(1, 2, 3)"
 PASS window.getComputedStyle(one).color is "rgb(3, 2, 1)"
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-05-10  Eric Seidel  <eric@webkit.org>
+
+        Make IFRAME_SEAMLESS child documents inherit styles from their parent iframe element
+        https://bugs.webkit.org/show_bug.cgi?id=85940
+
+        Reviewed by Ojan Vafai.
+
+        The HTML5 <iframe seamless> spec says:
+        In a CSS-supporting user agent: the user agent must, for the purpose of CSS property
+        inheritance only, treat the root element of the active document of the iframe
+        element's nested browsing context as being a child of the iframe element.
+        (Thus inherited properties on the root element of the document in the
+        iframe will inherit the computed values of those properties on the iframe
+        element instead of taking their initial values.)
+
+        Initially I implemented this support to the letter of the spec. However, doing so I learned
+        that WebKit has a RenderStyle for the Document Node, not just the root element of the document.
+        In this RenderStyle on the Document, we add a bunch of per-document styles from settings
+        including designMode.
+
+        This change makes StyleResolver::styleForDocument inherit style from the parent iframe's
+        style, before applying any of these per-document styles.  This may or may not be correct
+        depending on what behavior we want for rtl-ordering, page-zoom, locale, design mode, etc.
+        For now, we continue to treat the iframe's document as independent in these regards, and
+        the settings on that document override those inherited from the iframe.
+
+        Also, intially when making this work, I added redirects in recalcStyle and scheduleStyleRecalc
+        from the child document to the parent document in the case of seamless (since the parent
+        document effectively manages the style resolve and layout of the child in seamless mode).
+        However, I was not able to find a test which depended on this code change, so in this final patch
+        I have removed both of these modifications and replaced them with FIXMEs.  Based on discussions
+        with Ojan and James Robinson, I believe both of those changes may eventually be wanted.
+
+        This change basically does 3 things:
+        1.  Makes StyleResolver::styleForDocument inherit from the parent iframe.
+        2.  Makes any recalcStyle calls on the iframe propogate down into the child document (HTMLIFrameElement::didRecalcStyle).
+        3.  Makes Document::recalcStyle aware of the fact that the Document's style *can* change
+            for reasons other than recalcStyle(Force).
+
+        I'm open to more testing suggestions, if reviewers have settings on the Document's style
+        that you want to make sure we inherit from the parent iframe, or don't inherit, etc.
+        I view this as a complete solution to this aspect of the current <iframe seamless> spec,
+        but likely not the last code we will write for this aspect of the seamless feature. :)
+
+        Tested by fast/frames/seamlesss/seamless-css-cascade.html and seamless-designMode.html
+
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::collectMatchingRulesForList):
+        * dom/Document.cpp:
+        (WebCore::Document::scheduleStyleRecalc):
+        (WebCore::Document::recalcStyle):
+        * html/HTMLIFrameElement.cpp:
+        (WebCore::HTMLIFrameElement::HTMLIFrameElement):
+        (WebCore::HTMLIFrameElement::didRecalcStyle):
+        (WebCore):
+        * html/HTMLIFrameElement.h:
+        (HTMLIFrameElement):
+
 2012-05-10  Julien Chaffraix  <jchaffraix@webkit.org>
 

trunk/Source/WebCore/css/StyleResolver.cpp

@@ -1528 +1528 @@
     Frame* frame = document->frame();
 
+    // HTML5 states that seamless iframes should replace default CSS values
+    // with values inherited from the containing iframe element. However,
+    // some values (such as the case of designMode = "on") still need to
+    // be set by this "document style".
     RefPtr<RenderStyle> documentStyle = RenderStyle::create();
+    bool seamlessWithParent = document->shouldDisplaySeamlesslyWithParent();
+    if (seamlessWithParent) {
+        RenderStyle* iframeStyle = document->seamlessParentIFrame()->renderStyle();
+        if (iframeStyle)
+            documentStyle->inheritFrom(iframeStyle);
+    }
+
+    // FIXME: It's not clear which values below we want to override in the seamless case!
     documentStyle->setDisplay(BLOCK);
-    documentStyle->setRTLOrdering(document->visuallyOrdered() ? VisualOrder : LogicalOrder);
-    documentStyle->setZoom(frame && !document->printing() ? frame->pageZoomFactor() : 1);
-    documentStyle->setPageScaleTransform(frame ? frame->frameScaleFactor() : 1);
+    if (!seamlessWithParent) {
+        documentStyle->setRTLOrdering(document->visuallyOrdered() ? VisualOrder : LogicalOrder);
+        documentStyle->setZoom(frame && !document->printing() ? frame->pageZoomFactor() : 1);
+        documentStyle->setPageScaleTransform(frame ? frame->frameScaleFactor() : 1);
+        documentStyle->setLocale(document->contentLanguage());
+    }
+    // FIXME: This overrides any -webkit-user-modify inherited from the parent iframe.
     documentStyle->setUserModify(document->inDesignMode() ? READ_WRITE : READ_ONLY);
-    documentStyle->setLocale(document->contentLanguage());
 
     Element* docElement = document->documentElement();
@@ -1562 +1577 @@
         }
     }
+
+    // Seamless iframes want to inherit their font from their parent iframe, so early return before setting the font.
+    if (seamlessWithParent)
+        return documentStyle.release();
 
     FontDescription fontDescription;

trunk/Source/WebCore/dom/Document.cpp

@@ -1652 +1652 @@
 void Document::scheduleStyleRecalc()
 {
+    // FIXME: In the seamless case, we should likely schedule a style recalc
+    // on our parent and instead return early here.
+
     if (m_styleRecalcTimer.isActive() || inPageCache())
         return;
@@ -1705 +1708 @@
     if (m_inStyleRecalc)
         return; // Guard against re-entrancy. -dwh
-    
+
+    // FIXME: In the seamless case, we may wish to exit early in the child after recalcing our parent chain.
+    // I have not yet found a test which requires such.
+
     if (m_hasDirtyStyleResolver)
         updateActiveStylesheets(RecalcStyleImmediately);
@@ -1731 +1737 @@
         change = Force;
 
-    if (change == Force) {
+    // Recalculating the root style (on the document) is not needed in the common case.
+    if ((change == Force) || (shouldDisplaySeamlesslyWithParent() && (change >= Inherit))) {
         // style selector may set this again during recalc
         m_hasNodesWithPlaceholderStyle = false;

trunk/Source/WebCore/html/HTMLIFrameElement.cpp

@@ -42 +42 @@
 {
     ASSERT(hasTagName(iframeTag));
+    setHasCustomWillOrDidRecalcStyle();
 }
 
@@ -121 +122 @@
 }
 
+void HTMLIFrameElement::didRecalcStyle(StyleChange styleChange)
+{
+    if (!shouldDisplaySeamlessly())
+        return;
+    Document* childDocument = contentDocument();
+    if (styleChange >= Inherit || childDocument->childNeedsStyleRecalc() || childDocument->needsStyleRecalc())
+        contentDocument()->recalcStyle(styleChange);
+}
+
 #if ENABLE(MICRODATA)
 String HTMLIFrameElement::itemValueText() const

trunk/Source/WebCore/html/HTMLIFrameElement.h

@@ -44 +44 @@
     virtual InsertionNotificationRequest insertedInto(Node*) OVERRIDE;
     virtual void removedFrom(Node*) OVERRIDE;
-    
+
     virtual bool rendererIsNeeded(const NodeRenderingContext&);
     virtual RenderObject* createRenderer(RenderArena*, RenderStyle*);
-       
+
+    virtual void didRecalcStyle(StyleChange) OVERRIDE;
+
 #if ENABLE(MICRODATA)
     virtual String itemValueText() const OVERRIDE;