Changeset 94369 in webkit


Ignore:
Timestamp:
Sep 1, 2011 5:18:17 PM (13 years ago)
Author:
jchaffraix@webkit.org
Message:

REGRESSION (r84327-r84329): CSS stylesheets fail to load on www.flagstar.com login page
​https://bugs.webkit.org/show_bug.cgi?id=65140

Reviewed by Darin Adler.

Source/WebCore:

Tests: fast/css/stylesheet-enable-first-alternate-link.html

fast/css/stylesheet-enable-first-alternate-on-load-link.html
fast/css/stylesheet-enable-first-alternate-on-load-sheet.html
fast/css/stylesheet-enable-second-alternate-link.html
fast/css/stylesheet-enable-second-alternate-on-load-link.html
fast/css/stylesheet-enable-second-alternate-on-load-sheet.html
http/tests/css/link-css-disabled-value-with-slow-loading-sheet-in-error.html
http/tests/css/link-css-disabled-value-with-slow-loading-sheet.html

This patch basically reverts 88479 and 84329 while keeping the tests
we developped during the implementation.

Following discussion, it looks like HTML5 will need to be amended.
In the meantime, we will just revert the changes so that we can come
up with a better change.

  • dom/Document.cpp:

(WebCore::Document::recalcStyleSelector):

  • html/HTMLLinkElement.cpp:

(WebCore::HTMLLinkElement::HTMLLinkElement):
(WebCore::HTMLLinkElement::setDisabledState):
(WebCore::HTMLLinkElement::parseMappedAttribute):
(WebCore::HTMLLinkElement::process):
Revert those method to their original content.

  • html/HTMLLinkElement.h:

(WebCore::HTMLLinkElement::isDisabled):
(WebCore::HTMLLinkElement::isEnabledViaScript):
(WebCore::HTMLLinkElement::isAlternate):
Re-introduced the DisabledState enum.

  • html/HTMLLinkElement.idl: |disabled| is Reflect'ed again.

LayoutTests:

  • fast/css/stylesheet-enable-first-alternate-link-expected.txt: Added.
  • fast/css/stylesheet-enable-first-alternate-link.html: Added.
  • fast/css/stylesheet-enable-first-alternate-on-load-link-expected.txt: Added.
  • fast/css/stylesheet-enable-first-alternate-on-load-link.html: Added.
  • fast/css/stylesheet-enable-first-alternate-on-load-sheet-expected.txt: Added.
  • fast/css/stylesheet-enable-first-alternate-on-load-sheet.html: Added.
  • fast/css/stylesheet-enable-second-alternate-link-expected.txt: Added.
  • fast/css/stylesheet-enable-second-alternate-link.html: Added.
  • fast/css/stylesheet-enable-second-alternate-on-load-link-expected.txt: Added.
  • fast/css/stylesheet-enable-second-alternate-on-load-link.html: Added.
  • fast/css/stylesheet-enable-second-alternate-on-load-sheet-expected.txt: Added.
  • fast/css/stylesheet-enable-second-alternate-on-load-sheet.html: Added.
  • http/tests/css/link-css-disabled-value-with-slow-loading-sheet-expected.txt: Added.
  • http/tests/css/link-css-disabled-value-with-slow-loading-sheet-in-error-expected.txt: Added.
  • http/tests/css/link-css-disabled-value-with-slow-loading-sheet-in-error.html: Added.
  • http/tests/css/link-css-disabled-value-with-slow-loading-sheet.html: Added.
  • http/tests/css/resources/link-css-disabled-value-with-slow-loading-sheet-in-error.js: Added.

(onSheetLoaded):
(testWhenLoaded):

  • http/tests/css/resources/link-css-disabled-value-with-slow-loading-sheet.js: Added.

(onSheetLoaded):
(testWhenLoaded):

  • http/tests/css/resources/slow-loading-sheet-in-error.php: Added.
  • http/tests/css/resources/slow-loading-sheet.php: Added.

New tests developped for 65140. The baselines matches what FF and Opera are doing (IE does not support
link.sheet at the moment). Those failures are covered by bug 67436.

  • fast/css/link-disabled-attr-expected.txt:
  • fast/dom/HTMLLinkElement/disabled-attribute-expected.txt:

Fixed the baseline to match our behavior. The FAIL are our current deviation from the spec. Those
failures are covered by bug 67436.

  • fast/dom/boolean-attribute-reflection-expected.txt:
  • fast/dom/script-tests/boolean-attribute-reflection.js:

Re-added link.disabled as a reflected attribute.

Location:
trunk
Files:
20 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r94367 r94369  
     12011-09-01  Julien Chaffraix  <jchaffraix@webkit.org>
     2
     3        REGRESSION (r84327-r84329): CSS stylesheets fail to load on www.flagstar.com login page
     4        https://bugs.webkit.org/show_bug.cgi?id=65140
     5
     6        Reviewed by Darin Adler.
     7
     8        * fast/css/stylesheet-enable-first-alternate-link-expected.txt: Added.
     9        * fast/css/stylesheet-enable-first-alternate-link.html: Added.
     10        * fast/css/stylesheet-enable-first-alternate-on-load-link-expected.txt: Added.
     11        * fast/css/stylesheet-enable-first-alternate-on-load-link.html: Added.
     12        * fast/css/stylesheet-enable-first-alternate-on-load-sheet-expected.txt: Added.
     13        * fast/css/stylesheet-enable-first-alternate-on-load-sheet.html: Added.
     14        * fast/css/stylesheet-enable-second-alternate-link-expected.txt: Added.
     15        * fast/css/stylesheet-enable-second-alternate-link.html: Added.
     16        * fast/css/stylesheet-enable-second-alternate-on-load-link-expected.txt: Added.
     17        * fast/css/stylesheet-enable-second-alternate-on-load-link.html: Added.
     18        * fast/css/stylesheet-enable-second-alternate-on-load-sheet-expected.txt: Added.
     19        * fast/css/stylesheet-enable-second-alternate-on-load-sheet.html: Added.
     20        * http/tests/css/link-css-disabled-value-with-slow-loading-sheet-expected.txt: Added.
     21        * http/tests/css/link-css-disabled-value-with-slow-loading-sheet-in-error-expected.txt: Added.
     22        * http/tests/css/link-css-disabled-value-with-slow-loading-sheet-in-error.html: Added.
     23        * http/tests/css/link-css-disabled-value-with-slow-loading-sheet.html: Added.
     24        * http/tests/css/resources/link-css-disabled-value-with-slow-loading-sheet-in-error.js: Added.
     25        (onSheetLoaded):
     26        (testWhenLoaded):
     27        * http/tests/css/resources/link-css-disabled-value-with-slow-loading-sheet.js: Added.
     28        (onSheetLoaded):
     29        (testWhenLoaded):
     30        * http/tests/css/resources/slow-loading-sheet-in-error.php: Added.
     31        * http/tests/css/resources/slow-loading-sheet.php: Added.
     32        New tests developped for 65140. The baselines matches what FF and Opera are doing (IE does not support
     33        link.sheet at the moment). Those failures are covered by bug 67436.
     34
     35        * fast/css/link-disabled-attr-expected.txt:
     36        * fast/dom/HTMLLinkElement/disabled-attribute-expected.txt:
     37        Fixed the baseline to match our behavior. The FAIL are our current deviation from the spec. Those
     38        failures are covered by bug 67436.
     39
     40        * fast/dom/boolean-attribute-reflection-expected.txt:
     41        * fast/dom/script-tests/boolean-attribute-reflection.js:
     42        Re-added link.disabled as a reflected attribute.
     43
    1442011-09-01  Ryosuke Niwa  <rniwa@webkit.org>
    245

  • trunk/LayoutTests/fast/css/link-disabled-attr-expected.txt

    r93439 r94369  
    44notsheet
    55PASS link.sheet is null
    6 PASS link.disabled is false
     6FAIL link.disabled should be false. Was true.
    77sheet
    88PASS link.sheet is non-null.
    9 PASS link.disabled is true
     9FAIL link.disabled should be true. Was false.
    1010PASS link.sheet.disabled is true
    1111PASS getComputedStyle(console).whiteSpace is 'normal'
    1212PASS link.disabled is false
    13 PASS link.sheet.disabled is false
    14 PASS getComputedStyle(console).whiteSpace is 'pre-wrap'
     13FAIL link.sheet.disabled should be false. Was true.
     14FAIL getComputedStyle(console).whiteSpace should be pre-wrap. Was normal.
    1515altsheet
    1616FAIL link.disabled should be true. Was false.

  • trunk/LayoutTests/fast/dom/HTMLLinkElement/disabled-attribute-expected.txt

    r84329 r94369  
    66PASS test.disabled is false
    77PASS test.disabled is true
    8 PASS test.sheet.disabled is true
     8FAIL test.sheet.disabled should be true. Was false.
    99PASS test.sheet.disabled is false
    10 PASS test.disabled is false
     10FAIL test.disabled should be false. Was true.
    1111
    1212
    1313PASS test_nostyle.sheet is null
    1414PASS test_nostyle.disabled is false
    15 PASS test_nostyle.disabled is false
     15FAIL test_nostyle.disabled should be false. Was true.
    1616PASS successfullyParsed is true
    1717

  • trunk/LayoutTests/fast/dom/boolean-attribute-reflection-expected.txt

    r84329 r94369  
    6060PASS e = make('input'); e.setAttribute('required', 'x'); e.required = false; e.getAttribute('required') is null
    6161PASS e = make('input'); e.setAttribute('required', 'x'); e.required = true; e.getAttribute('required') is ''
     62PASS e = make('link'); e.removeAttribute('disabled'); e.disabled is false
     63PASS e = make('link'); e.setAttribute('disabled', ''); e.disabled is true
     64PASS e = make('link'); e.setAttribute('disabled', 'x'); e.disabled = false; e.getAttribute('disabled') is null
     65PASS e = make('link'); e.setAttribute('disabled', 'x'); e.disabled = true; e.getAttribute('disabled') is ''
    6266PASS e = make('menu'); e.removeAttribute('compact'); e.compact is false
    6367PASS e = make('menu'); e.setAttribute('compact', ''); e.compact is true

  • trunk/LayoutTests/fast/dom/script-tests/boolean-attribute-reflection.js

    r84329 r94369  
    1616    [ "input", "readOnly" ],
    1717    [ "input", "required" ],
     18    [ "link", "disabled" ],
    1819    [ "menu", "compact" ],
    1920    [ "object", "declare" ],

  • trunk/Source/WebCore/ChangeLog

    r94368 r94369  
     12011-09-01  Julien Chaffraix  <jchaffraix@webkit.org>
     2
     3        REGRESSION (r84327-r84329): CSS stylesheets fail to load on www.flagstar.com login page
     4        https://bugs.webkit.org/show_bug.cgi?id=65140
     5
     6        Reviewed by Darin Adler.
     7
     8        Tests: fast/css/stylesheet-enable-first-alternate-link.html
     9               fast/css/stylesheet-enable-first-alternate-on-load-link.html
     10               fast/css/stylesheet-enable-first-alternate-on-load-sheet.html
     11               fast/css/stylesheet-enable-second-alternate-link.html
     12               fast/css/stylesheet-enable-second-alternate-on-load-link.html
     13               fast/css/stylesheet-enable-second-alternate-on-load-sheet.html
     14               http/tests/css/link-css-disabled-value-with-slow-loading-sheet-in-error.html
     15               http/tests/css/link-css-disabled-value-with-slow-loading-sheet.html
     16
     17        This patch basically reverts 88479 and 84329 while keeping the tests
     18        we developped during the implementation.
     19
     20        Following discussion, it looks like HTML5 will need to be amended.
     21        In the meantime, we will just revert the changes so that we can come
     22        up with a better change.
     23
     24        * dom/Document.cpp:
     25        (WebCore::Document::recalcStyleSelector):
     26        * html/HTMLLinkElement.cpp:
     27        (WebCore::HTMLLinkElement::HTMLLinkElement):
     28        (WebCore::HTMLLinkElement::setDisabledState):
     29        (WebCore::HTMLLinkElement::parseMappedAttribute):
     30        (WebCore::HTMLLinkElement::process):
     31        Revert those method to their original content.
     32
     33        * html/HTMLLinkElement.h:
     34        (WebCore::HTMLLinkElement::isDisabled):
     35        (WebCore::HTMLLinkElement::isEnabledViaScript):
     36        (WebCore::HTMLLinkElement::isAlternate):
     37        Re-introduced the DisabledState enum.
     38
     39        * html/HTMLLinkElement.idl: |disabled| is Reflect'ed again.
     40
    1412011-09-01  Dan Bernstein  <mitz@apple.com>
    242

  • trunk/Source/WebCore/dom/Document.cpp

    r94290 r94369  
    29602960                // <LINK> element
    29612961                HTMLLinkElement* linkElement = static_cast<HTMLLinkElement*>(n);
    2962                 if (linkElement->disabled())
     2962                if (linkElement->isDisabled())
    29632963                    continue;
    29642964                enabledViaScript = linkElement->isEnabledViaScript();

  • trunk/Source/WebCore/html/HTMLLinkElement.cpp

    r94213 r94369  
    5656    , m_linkLoader(this)
    5757    , m_sizes(DOMSettableTokenList::create())
     58    , m_disabledState(Unset)
    5859    , m_loading(false)
    59     , m_isEnabledViaScript(false)
    6060    , m_createdByParser(createdByParser)
    6161    , m_isInShadowTree(false)
    … …  
    8484}
    8585
    86 void HTMLLinkElement::setDisabled(bool disabled)
    87 {
    88     if (!m_sheet)
    89         return;
    90 
    91     bool wasDisabled = m_sheet->disabled();
    92     if (wasDisabled == disabled)
    93         return;
    94 
    95     m_sheet->setDisabled(disabled);
    96     m_isEnabledViaScript = !disabled;
    97 
    98     // If we change the disabled state while the sheet is still loading, then we have to
    99     // perform three checks:
    100     if (isLoading()) {
    101         // Check #1: The sheet becomes disabled while loading.
    102         if (disabled)
    103             removePendingSheet();
    104 
    105         // Check #2: An alternate sheet becomes enabled while it is still loading.
    106         if (m_relAttribute.m_isAlternate && !disabled)
    107             addPendingSheet(Blocking);
    108 
    109         // Check #3: A main sheet becomes enabled while it was still loading and
    110         // after it was disabled via script. It takes really terrible code to make this
    111         // happen (a double toggle for no reason essentially). This happens on
    112         // virtualplastic.net, which manages to do about 12 enable/disables on only 3
    113         // sheets. :)
    114         if (!m_relAttribute.m_isAlternate && !disabled && wasDisabled)
    115             addPendingSheet(Blocking);
    116 
    117         // If the sheet is already loading just bail.
    118         return;
    119     }
    120 
    121     if (!disabled)
    122         process();
     86void HTMLLinkElement::setDisabledState(bool disabled)
     87{
     88    DisabledState oldDisabledState = m_disabledState;
     89    m_disabledState = disabled ? Disabled : EnabledViaScript;
     90    if (oldDisabledState != m_disabledState) {
     91        // If we change the disabled state while the sheet is still loading, then we have to
     92        // perform three checks:
     93        if (isLoading()) {
     94            // Check #1: The sheet becomes disabled while loading.
     95            if (m_disabledState == Disabled)
     96                removePendingSheet();
     97
     98            // Check #2: An alternate sheet becomes enabled while it is still loading.
     99            if (m_relAttribute.m_isAlternate && m_disabledState == EnabledViaScript)
     100                addPendingSheet(Blocking);
     101
     102            // Check #3: A main sheet becomes enabled while it was still loading and
     103            // after it was disabled via script. It takes really terrible code to make this
     104            // happen (a double toggle for no reason essentially). This happens on
     105            // virtualplastic.net, which manages to do about 12 enable/disables on only 3
     106            // sheets. :)
     107            if (!m_relAttribute.m_isAlternate && m_disabledState == EnabledViaScript && oldDisabledState == Disabled)
     108                addPendingSheet(Blocking);
     109
     110            // If the sheet is already loading just bail.
     111            return;
     112        }
     113
     114        // Load the sheet, since it's never been loaded before.
     115        if (!m_sheet && m_disabledState == EnabledViaScript)
     116            process();
     117        else
     118            document()->styleSelectorChanged(DeferRecalcStyle); // Update the style selector.
     119    }
    123120}
    124121
    … …  
    146143        m_media = attr->value().string().lower();
    147144        process();
    148     } else if (attr->name() == onbeforeloadAttr)
     145    } else if (attr->name() == disabledAttr)
     146        setDisabledState(!attr->isNull());
     147    else if (attr->name() == onbeforeloadAttr)
    149148        setAttributeEventListener(eventNames().beforeloadEvent, createAttributeEventListener(this, attr));
    150149#if ENABLE(LINK_PREFETCH)
    … …  
    186185    bool acceptIfTypeContainsTextCSS = document()->page() && document()->page()->settings() && document()->page()->settings()->treatsAnyTextCSSLinkAsStylesheet();
    187186
    188     if (!disabled() && (m_relAttribute.m_isStyleSheet || (acceptIfTypeContainsTextCSS && type.contains("text/css")))
     187    if (m_disabledState != Disabled && (m_relAttribute.m_isStyleSheet || (acceptIfTypeContainsTextCSS && type.contains("text/css")))
    189188        && document()->frame() && m_url.isValid()) {
    190189       
    … …  
    445444}
    446445
    447 bool HTMLLinkElement::disabled() const
    448 {
    449     return m_sheet && m_sheet->disabled();
    450 }
    451 
    452446DOMSettableTokenList* HTMLLinkElement::sizes() const
    453447{

  • trunk/Source/WebCore/html/HTMLLinkElement.h

    r93439 r94369  
    5858    // FIXME: This should be renamed isStyleSheetLoading as this is only used for stylesheets.
    5959    bool isLoading() const;
    60     bool isEnabledViaScript() const { return m_isEnabledViaScript; }
    61     bool disabled() const;
    62     void setDisabled(bool);
     60
     61    bool isDisabled() const { return m_disabledState == Disabled; }
     62    bool isEnabledViaScript() const { return m_disabledState == EnabledViaScript; }
    6363    void setSizes(const String&);
    6464    DOMSettableTokenList* sizes() const;
    … …  
    8282    virtual void linkLoadingErrored();
    8383
    84     bool isAlternate() const { return m_relAttribute.m_isAlternate; }
     84    bool isAlternate() const { return m_disabledState == Unset && m_relAttribute.m_isAlternate; }
    8585   
     86    void setDisabledState(bool);
     87
    8688    virtual bool isURLAttribute(Attribute*) const;
    8789
    … …  
    101103    CachedResourceHandle<CachedCSSStyleSheet> m_cachedSheet;
    102104    RefPtr<CSSStyleSheet> m_sheet;
     105    enum DisabledState {
     106        Unset,
     107        EnabledViaScript,
     108        Disabled
     109    };
     110
    103111    KURL m_url;
    104112    String m_type;
    105113    String m_media;
    106114    RefPtr<DOMSettableTokenList> m_sizes;
     115    DisabledState m_disabledState;
    107116    LinkRelAttribute m_relAttribute;
    108117    bool m_loading;
    109     bool m_isEnabledViaScript;
    110118    bool m_createdByParser;
    111119    bool m_isInShadowTree;

  • trunk/Source/WebCore/html/HTMLLinkElement.idl

    r91893 r94369  
    2323
    2424    interface HTMLLinkElement : HTMLElement {
    25         attribute boolean disabled;
     25        attribute [Reflect] boolean disabled;
    2626        attribute [Reflect] DOMString charset;
    2727        attribute [Reflect, URL] DOMString href;
Note: See TracChangeset for help on using the changeset viewer.