Title: [104759] branches/safari-534.54-branch

Diff

Modified: branches/safari-534.54-branch/LayoutTests/ChangeLog (104758 => 104759)


--- branches/safari-534.54-branch/LayoutTests/ChangeLog	2012-01-11 23:57:39 UTC (rev 104758)
+++ branches/safari-534.54-branch/LayoutTests/ChangeLog	2012-01-12 00:02:15 UTC (rev 104759)
@@ -1,3 +1,21 @@
+2011-1-11  Lucas Forschler  <lforsch...@apple.com>
+
+    Merge 88479
+
+    2011-06-09  Julien Chaffraix  <jchaffr...@codeaurora.org>
+
+            Reviewed by Antti Koivisto.
+
+            REGRESSION(84329): Stylesheets on some pages do not load
+            https://bugs.webkit.org/show_bug.cgi?id=61400
+
+            Adding test to cover the regression. The test actually uncovered
+            a bug in the way we handle alternate stylesheet and thus is
+            failing some parts.
+
+            * fast/css/link-disabled-attr-expected.txt: Added.
+            * fast/css/link-disabled-attr.html: Added.
+
 2011-1-9  Lucas Forschler  <lforsch...@apple.com>
 
     Merge 102509

Copied: branches/safari-534.54-branch/LayoutTests/fast/css/link-disabled-attr-expected.txt (from rev 88479, trunk/LayoutTests/fast/css/link-disabled-attr-expected.txt) (0 => 104759)


--- branches/safari-534.54-branch/LayoutTests/fast/css/link-disabled-attr-expected.txt	                        (rev 0)
+++ branches/safari-534.54-branch/LayoutTests/fast/css/link-disabled-attr-expected.txt	2012-01-12 00:02:15 UTC (rev 104759)
@@ -0,0 +1,27 @@
+Series of tests to validate behavior of getting/setting link.disabled and link.sheet.disabled.
+Test for bug 61400: REGRESSION(84329): Stylesheets on some pages do not load
+
+notsheet
+PASS link.sheet is null
+PASS link.disabled is false
+sheet
+PASS link.sheet is non-null.
+PASS link.disabled is true
+PASS link.sheet.disabled is true
+PASS getComputedStyle(console).whiteSpace is 'normal'
+PASS link.disabled is false
+PASS link.sheet.disabled is false
+PASS getComputedStyle(console).whiteSpace is 'pre-wrap'
+altsheet
+FAIL link.disabled should be true. Was false.
+PASS link.sheet is non-null.
+FAIL getComputedStyle(console).backgroundColor should be rgb(0, 128, 0). Was rgba(0, 0, 0, 0).
+FAIL link.disabled should be true. Was false.
+PASS getComputedStyle(console).backgroundColor is originalBG
+PASS link.disabled is false
+FAIL getComputedStyle(console).backgroundColor should be rgb(0, 128, 0). Was rgba(0, 0, 0, 0).
+PASS getComputedStyle(console).backgroundColor is originalBG
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Copied: branches/safari-534.54-branch/LayoutTests/fast/css/link-disabled-attr.html (from rev 88479, trunk/LayoutTests/fast/css/link-disabled-attr.html) (0 => 104759)


--- branches/safari-534.54-branch/LayoutTests/fast/css/link-disabled-attr.html	                        (rev 0)
+++ branches/safari-534.54-branch/LayoutTests/fast/css/link-disabled-attr.html	2012-01-12 00:02:15 UTC (rev 104759)
@@ -0,0 +1,103 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link id="sheet" rel="stylesheet" href=""
+<link id="notsheet" rel="author" href=""
+<link id="alt" rel="alternate stylesheet" title="altset" href=""
+<script src=""
+</head>
+<body>
+<p id="description">
+Series of tests to validate behavior of getting/setting link.disabled and link.sheet.disabled.<br>
+Test for bug <a href="" REGRESSION(84329): Stylesheets on some pages do not load
+</p>
+<div id="console"></div>
+<script>
+
+window.jsTestIsAsync = true;
+
+function onSheetLoaded(f, elem, maxtime) {
+    if (elem.sheet || maxtime <= 0)
+        f(elem);
+    else
+        setTimeout(function () { onSheetLoaded(f, elem, maxtime - 25);}, 25);
+}
+
+
+
+// With a non-stylesheet <link>, 'disabled' is always false.
+
+var console = document.getElementById("console");
+var originalBG = getComputedStyle(console).backgroundColor;
+var link;
+
+debug("notsheet");
+
+link = document.getElementById("notsheet");
+shouldBeNull("link.sheet");
+link.disabled = true;
+shouldBeFalse("link.disabled");
+
+
+// With a stylesheet <link>, 'disabled' and 'link.style.disabled' should both
+// work, and be consistent with each other.
+
+debug("sheet");
+
+link = document.getElementById("sheet");
+shouldBeNonNull("link.sheet");
+
+link.sheet.disabled = true;
+shouldBeTrue("link.disabled");
+shouldBeTrue("link.sheet.disabled");
+shouldBe("getComputedStyle(console).whiteSpace", "'normal'");
+
+link.disabled = false;
+shouldBeFalse("link.disabled");
+shouldBeFalse("link.sheet.disabled");
+shouldBe("getComputedStyle(console).whiteSpace", "'pre-wrap'");
+
+link.sheet.disabled = false;
+
+
+// An alternate stylesheet defaults to disabled when its title does not
+// match the preferred set.
+
+debug("altsheet");
+link = document.getElementById("alt");
+shouldBeTrue("link.disabled");
+
+// Toggling link.disabled activates the stylesheet.
+
+function altSheetLoaded(e) {
+    link = e;
+    shouldBeNonNull("link.sheet");
+    shouldBe("getComputedStyle(console).backgroundColor", "'rgb(0, 128, 0)'");
+
+    // Enabling a stylsheet set modifies disabled status of style sheets.
+
+    document.selectedStyleSheetSet = "nosuchset";
+    shouldBeTrue("link.disabled");
+    shouldBe("getComputedStyle(console).backgroundColor", "originalBG");
+
+    document.selectedStyleSheetSet = "altset";
+    shouldBeFalse("link.disabled");
+    shouldBe("getComputedStyle(console).backgroundColor", "'rgb(0, 128, 0)'");
+
+    // Disabling a stylesheet *after* its stylesheet set has been selected
+    // de-activates it.
+
+    link.disabled = true;
+    shouldBe("getComputedStyle(console).backgroundColor", "originalBG");
+
+    finishJSTest();
+}
+
+link.disabled = false;
+onSheetLoaded(altSheetLoaded, link, 500);
+
+successfullyParsed = true;
+
+</script>
+<script src=""
+</body></html>

Modified: branches/safari-534.54-branch/Source/WebCore/ChangeLog (104758 => 104759)


--- branches/safari-534.54-branch/Source/WebCore/ChangeLog	2012-01-11 23:57:39 UTC (rev 104758)
+++ branches/safari-534.54-branch/Source/WebCore/ChangeLog	2012-01-12 00:02:15 UTC (rev 104759)
@@ -1,3 +1,41 @@
+2011-1-11  Lucas Forschler  <lforsch...@apple.com>
+
+    Merge 88479
+
+    2011-06-09  Julien Chaffraix  <jchaffr...@codeaurora.org>
+
+            Reviewed by Antti Koivisto.
+
+            REGRESSION(84329): Stylesheets on some pages do not load
+            https://bugs.webkit.org/show_bug.cgi?id=61400
+
+            Test: fast/css/link-disabled-attr.html
+
+            Fixed r84329: the change did not take into account the fact
+            that HTMLLinkElement did already contain the disabled information
+            and the 2 information were not linked as they should have!
+
+            The new logic pushes the information to the stylesheet as this
+            is what the spec mandates and what FF is doing. Also it keeps
+            one bit of information (that JS enabled the stylesheet) as it
+            is needed for the recalcStyleSelector logic.
+
+            * dom/Document.cpp:
+            (WebCore::Document::recalcStyleSelector): s/isDisabled/disabled.
+
+            * html/HTMLLinkElement.cpp:
+            (WebCore::HTMLLinkElement::HTMLLinkElement): Removed m_disabledState,
+            replaced by m_isEnabledViaScript.
+            (WebCore::HTMLLinkElement::setDisabled): Updated the logic after
+            m_disabledState removal. It also matches the spec by forwarding
+            the disabled state to our stylesheet if we have one.
+            (WebCore::HTMLLinkElement::parseMappedAttribute): Removed harmful
+            handling of the disabledAttr.
+            (WebCore::HTMLLinkElement::process): Updated after m_disabledState removal.
+            * html/HTMLLinkElement.h:
+            (WebCore::HTMLLinkElement::isEnabledViaScript): Ditto.
+            (WebCore::HTMLLinkElement::isAlternate): Ditto.
+
 2011-1-9  Lucas Forschler  <lforsch...@apple.com>
 
     Merge 102263

Modified: branches/safari-534.54-branch/Source/WebCore/dom/Document.cpp (104758 => 104759)


--- branches/safari-534.54-branch/Source/WebCore/dom/Document.cpp	2012-01-11 23:57:39 UTC (rev 104758)
+++ branches/safari-534.54-branch/Source/WebCore/dom/Document.cpp	2012-01-12 00:02:15 UTC (rev 104759)
@@ -3094,7 +3094,7 @@
             if (e->hasLocalName(linkTag)) {
                 // <LINK> element
                 HTMLLinkElement* linkElement = static_cast<HTMLLinkElement*>(n);
-                if (linkElement->isDisabled())
+                if (linkElement->disabled())
                     continue;
                 enabledViaScript = linkElement->isEnabledViaScript();
                 if (linkElement->isLoading()) {

Modified: branches/safari-534.54-branch/Source/WebCore/html/HTMLLinkElement.cpp (104758 => 104759)


--- branches/safari-534.54-branch/Source/WebCore/html/HTMLLinkElement.cpp	2012-01-11 23:57:39 UTC (rev 104758)
+++ branches/safari-534.54-branch/Source/WebCore/html/HTMLLinkElement.cpp	2012-01-12 00:02:15 UTC (rev 104759)
@@ -55,8 +55,8 @@
 #if ENABLE(LINK_PREFETCH)
     , m_onloadTimer(this, &HTMLLinkElement::onloadTimerFired)
 #endif
-    , m_disabledState(Unset)
     , m_loading(false)
+    , m_isEnabledViaScript(false)
     , m_createdByParser(createdByParser)
     , m_isInShadowTree(false)
     , m_pendingSheetType(None)
@@ -88,40 +88,43 @@
 #endif
 }
 
-void HTMLLinkElement::setDisabledState(bool _disabled)
+void HTMLLinkElement::setDisabled(bool disabled)
 {
-    DisabledState oldDisabledState = m_disabledState;
-    m_disabledState = _disabled ? Disabled : EnabledViaScript;
-    if (oldDisabledState != m_disabledState) {
-        // If we change the disabled state while the sheet is still loading, then we have to
-        // perform three checks:
-        if (isLoading()) {
-            // Check #1: The sheet becomes disabled while loading.
-            if (m_disabledState == Disabled)
-                removePendingSheet();
+    if (!m_sheet)
+        return;
 
-            // Check #2: An alternate sheet becomes enabled while it is still loading.
-            if (m_relAttribute.m_isAlternate && m_disabledState == EnabledViaScript)
-                addPendingSheet(Blocking);
+    bool wasDisabled = m_sheet->disabled();
+    if (wasDisabled == disabled)
+        return;
 
-            // Check #3: A main sheet becomes enabled while it was still loading and
-            // after it was disabled via script.  It takes really terrible code to make this
-            // happen (a double toggle for no reason essentially).  This happens on
-            // virtualplastic.net, which manages to do about 12 enable/disables on only 3
-            // sheets. :)
-            if (!m_relAttribute.m_isAlternate && m_disabledState == EnabledViaScript && oldDisabledState == Disabled)
-                addPendingSheet(Blocking);
+    m_sheet->setDisabled(disabled);
+    m_isEnabledViaScript = !disabled;
 
-            // If the sheet is already loading just bail.
-            return;
-        }
+    // If we change the disabled state while the sheet is still loading, then we have to
+    // perform three checks:
+    if (isLoading()) {
+        // Check #1: The sheet becomes disabled while loading.
+        if (disabled)
+            removePendingSheet();
 
-        // Load the sheet, since it's never been loaded before.
-        if (!m_sheet && m_disabledState == EnabledViaScript)
-            process();
-        else
-            document()->styleSelectorChanged(DeferRecalcStyle); // Update the style selector.
+        // Check #2: An alternate sheet becomes enabled while it is still loading.
+        if (m_relAttribute.m_isAlternate && !disabled)
+            addPendingSheet(Blocking);
+
+        // Check #3: A main sheet becomes enabled while it was still loading and
+        // after it was disabled via script. It takes really terrible code to make this
+        // happen (a double toggle for no reason essentially). This happens on
+        // virtualplastic.net, which manages to do about 12 enable/disables on only 3
+        // sheets. :)
+        if (!m_relAttribute.m_isAlternate && !disabled && wasDisabled)
+            addPendingSheet(Blocking);
+
+        // If the sheet is already loading just bail.
+        return;
     }
+
+    if (!disabled)
+        process();
 }
 
 StyleSheet* HTMLLinkElement::sheet() const
@@ -143,9 +146,7 @@
     } else if (attr->name() == mediaAttr) {
         m_media = attr->value().string().lower();
         process();
-    } else if (attr->name() == disabledAttr)
-        setDisabledState(!attr->isNull());
-    else if (attr->name() == onbeforeloadAttr)
+    } else if (attr->name() == onbeforeloadAttr)
         setAttributeEventListener(eventNames().beforeloadEvent, createAttributeEventListener(this, attr));
 #if ENABLE(LINK_PREFETCH)
     else if (attr->name() == onloadAttr)
@@ -267,7 +268,7 @@
 
     bool acceptIfTypeContainsTextCSS = document()->page() && document()->page()->settings() && document()->page()->settings()->treatsAnyTextCSSLinkAsStylesheet();
     
-    if (m_disabledState != Disabled && (m_relAttribute.m_isStyleSheet || (acceptIfTypeContainsTextCSS && type.contains("text/css"))) 
+    if (!disabled() && (m_relAttribute.m_isStyleSheet || (acceptIfTypeContainsTextCSS && type.contains("text/css")))
         && document()->frame() && m_url.isValid()) {
         
         String charset = getAttribute(charsetAttr);
@@ -533,11 +534,4 @@
     return m_sheet && m_sheet->disabled();
 }
 
-void HTMLLinkElement::setDisabled(bool disabled)
-{
-    if (!m_sheet)
-        return;
-    m_sheet->setDisabled(disabled);
 }
-
-}

Modified: branches/safari-534.54-branch/Source/WebCore/html/HTMLLinkElement.h (104758 => 104759)


--- branches/safari-534.54-branch/Source/WebCore/html/HTMLLinkElement.h	2012-01-11 23:57:39 UTC (rev 104758)
+++ branches/safari-534.54-branch/Source/WebCore/html/HTMLLinkElement.h	2012-01-12 00:02:15 UTC (rev 104759)
@@ -73,10 +73,9 @@
 
     StyleSheet* sheet() const;
 
+    // FIXME: This should be remaned isStyleSheetLoading as this is only used for stylesheets.
     bool isLoading() const;
-
-    bool isDisabled() const { return m_disabledState == Disabled; }
-    bool isEnabledViaScript() const { return m_disabledState == EnabledViaScript; }
+    bool isEnabledViaScript() const { return m_isEnabledViaScript; }
     bool disabled() const;
     void setDisabled(bool);
 
@@ -100,10 +99,8 @@
 #endif
     virtual bool sheetLoaded();
 
-    bool isAlternate() const { return m_disabledState == Unset && m_relAttribute.m_isAlternate; }
+    bool isAlternate() const { return m_relAttribute.m_isAlternate; }
     
-    void setDisabledState(bool _disabled);
-
     virtual bool isURLAttribute(Attribute*) const;
 
 public:
@@ -121,12 +118,6 @@
 private:
     HTMLLinkElement(const QualifiedName&, Document*, bool createdByParser);
 
-    enum DisabledState {
-        Unset,
-        EnabledViaScript,
-        Disabled
-    };
-
     CachedResourceHandle<CachedCSSStyleSheet> m_cachedSheet;
     RefPtr<CSSStyleSheet> m_sheet;
 #if ENABLE(LINK_PREFETCH)
@@ -136,9 +127,9 @@
     KURL m_url;
     String m_type;
     String m_media;
-    DisabledState m_disabledState;
     RelAttribute m_relAttribute;
     bool m_loading;
+    bool m_isEnabledViaScript;
     bool m_createdByParser;
     bool m_isInShadowTree;
     
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to