Diff
Modified: trunk/LayoutTests/ChangeLog (88478 => 88479)
--- trunk/LayoutTests/ChangeLog 2011-06-09 20:30:39 UTC (rev 88478)
+++ trunk/LayoutTests/ChangeLog 2011-06-09 20:33:20 UTC (rev 88479)
@@ -1,3 +1,17 @@
+2011-06-09 Julien Chaffraix <[email protected]>
+
+ 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-06-09 Julien Chaffraix <[email protected]>
Reviewed by Darin Adler.
Added: trunk/LayoutTests/fast/css/link-disabled-attr-expected.txt (0 => 88479)
--- trunk/LayoutTests/fast/css/link-disabled-attr-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/link-disabled-attr-expected.txt 2011-06-09 20:33:20 UTC (rev 88479)
@@ -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
+
Added: trunk/LayoutTests/fast/css/link-disabled-attr.html (0 => 88479)
--- trunk/LayoutTests/fast/css/link-disabled-attr.html (rev 0)
+++ trunk/LayoutTests/fast/css/link-disabled-attr.html 2011-06-09 20:33:20 UTC (rev 88479)
@@ -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: trunk/Source/WebCore/ChangeLog (88478 => 88479)
--- trunk/Source/WebCore/ChangeLog 2011-06-09 20:30:39 UTC (rev 88478)
+++ trunk/Source/WebCore/ChangeLog 2011-06-09 20:33:20 UTC (rev 88479)
@@ -1,3 +1,37 @@
+2011-06-09 Julien Chaffraix <[email protected]>
+
+ 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-06-09 Dan Bernstein <[email protected]>
Reviewed by Darin Adler.
Modified: trunk/Source/WebCore/dom/Document.cpp (88478 => 88479)
--- trunk/Source/WebCore/dom/Document.cpp 2011-06-09 20:30:39 UTC (rev 88478)
+++ trunk/Source/WebCore/dom/Document.cpp 2011-06-09 20:33:20 UTC (rev 88479)
@@ -2967,7 +2967,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: trunk/Source/WebCore/html/HTMLLinkElement.cpp (88478 => 88479)
--- trunk/Source/WebCore/html/HTMLLinkElement.cpp 2011-06-09 20:30:39 UTC (rev 88478)
+++ trunk/Source/WebCore/html/HTMLLinkElement.cpp 2011-06-09 20:33:20 UTC (rev 88479)
@@ -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)
@@ -85,40 +85,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
@@ -140,9 +143,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)
@@ -279,7 +280,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);
@@ -554,11 +555,4 @@
return m_sheet && m_sheet->disabled();
}
-void HTMLLinkElement::setDisabled(bool disabled)
-{
- if (!m_sheet)
- return;
- m_sheet->setDisabled(disabled);
}
-
-}
Modified: trunk/Source/WebCore/html/HTMLLinkElement.h (88478 => 88479)
--- trunk/Source/WebCore/html/HTMLLinkElement.h 2011-06-09 20:30:39 UTC (rev 88478)
+++ trunk/Source/WebCore/html/HTMLLinkElement.h 2011-06-09 20:33:20 UTC (rev 88479)
@@ -75,10 +75,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);
@@ -103,10 +102,8 @@
virtual bool sheetLoaded();
virtual void startLoadingDynamicSheet();
- 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:
@@ -124,12 +121,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)
@@ -139,9 +130,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;
