Changeset 88479 in webkit


Ignore:
Timestamp:
Jun 9, 2011 1:33:20 PM (13 years ago)
Author:
commit-queue@webkit.org
Message:

2011-06-09 Julien Chaffraix <​jchaffraix@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-06-09 Julien Chaffraix <​jchaffraix@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.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r88474 r88479  
     12011-06-09  Julien Chaffraix  <jchaffraix@codeaurora.org>
     2
     3        Reviewed by Antti Koivisto.
     4
     5        REGRESSION(84329): Stylesheets on some pages do not load
     6        https://bugs.webkit.org/show_bug.cgi?id=61400
     7
     8        Adding test to cover the regression. The test actually uncovered
     9        a bug in the way we handle alternate stylesheet and thus is
     10        failing some parts.
     11
     12        * fast/css/link-disabled-attr-expected.txt: Added.
     13        * fast/css/link-disabled-attr.html: Added.
     14
    1152011-06-09  Julien Chaffraix  <jchaffraix@webkit.org>
    216

  • trunk/Source/WebCore/ChangeLog

    r88478 r88479  
     12011-06-09  Julien Chaffraix  <jchaffraix@codeaurora.org>
     2
     3        Reviewed by Antti Koivisto.
     4
     5        REGRESSION(84329): Stylesheets on some pages do not load
     6        https://bugs.webkit.org/show_bug.cgi?id=61400
     7
     8        Test: fast/css/link-disabled-attr.html
     9
     10        Fixed r84329: the change did not take into account the fact
     11        that HTMLLinkElement did already contain the disabled information
     12        and the 2 information were not linked as they should have!
     13
     14        The new logic pushes the information to the stylesheet as this
     15        is what the spec mandates and what FF is doing. Also it keeps
     16        one bit of information (that JS enabled the stylesheet) as it
     17        is needed for the recalcStyleSelector logic.
     18
     19        * dom/Document.cpp:
     20        (WebCore::Document::recalcStyleSelector): s/isDisabled/disabled.
     21
     22        * html/HTMLLinkElement.cpp:
     23        (WebCore::HTMLLinkElement::HTMLLinkElement): Removed m_disabledState,
     24        replaced by m_isEnabledViaScript.
     25        (WebCore::HTMLLinkElement::setDisabled): Updated the logic after
     26        m_disabledState removal. It also matches the spec by forwarding
     27        the disabled state to our stylesheet if we have one.
     28        (WebCore::HTMLLinkElement::parseMappedAttribute): Removed harmful
     29        handling of the disabledAttr.
     30        (WebCore::HTMLLinkElement::process): Updated after m_disabledState removal.
     31        * html/HTMLLinkElement.h:
     32        (WebCore::HTMLLinkElement::isEnabledViaScript): Ditto.
     33        (WebCore::HTMLLinkElement::isAlternate): Ditto.
     34
    1352011-06-09  Dan Bernstein  <mitz@apple.com>
    236

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

    r88242 r88479  
    29682968                // <LINK> element
    29692969                HTMLLinkElement* linkElement = static_cast<HTMLLinkElement*>(n);
    2970                 if (linkElement->isDisabled())
     2970                if (linkElement->disabled())
    29712971                    continue;
    29722972                enabledViaScript = linkElement->isEnabledViaScript();

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

    r87867 r88479  
    5656    , m_onloadTimer(this, &HTMLLinkElement::onloadTimerFired)
    5757#endif
    58     , m_disabledState(Unset)
    5958    , m_loading(false)
     59    , m_isEnabledViaScript(false)
    6060    , m_createdByParser(createdByParser)
    6161    , m_isInShadowTree(false)
    … …  
    8686}
    8787
    88 void HTMLLinkElement::setDisabledState(bool _disabled)
    89 {
    90     DisabledState oldDisabledState = m_disabledState;
    91     m_disabledState = _disabled ? Disabled : EnabledViaScript;
    92     if (oldDisabledState != m_disabledState) {
    93         // If we change the disabled state while the sheet is still loading, then we have to
    94         // perform three checks:
    95         if (isLoading()) {
    96             // Check #1: The sheet becomes disabled while loading.
    97             if (m_disabledState == Disabled)
    98                 removePendingSheet();
    99 
    100             // Check #2: An alternate sheet becomes enabled while it is still loading.
    101             if (m_relAttribute.m_isAlternate && m_disabledState == EnabledViaScript)
    102                 addPendingSheet(Blocking);
    103 
    104             // Check #3: A main sheet becomes enabled while it was still loading and
    105             // after it was disabled via script.  It takes really terrible code to make this
    106             // happen (a double toggle for no reason essentially).  This happens on
    107             // virtualplastic.net, which manages to do about 12 enable/disables on only 3
    108             // sheets. :)
    109             if (!m_relAttribute.m_isAlternate && m_disabledState == EnabledViaScript && oldDisabledState == Disabled)
    110                 addPendingSheet(Blocking);
    111 
    112             // If the sheet is already loading just bail.
    113             return;
    114         }
    115 
    116         // Load the sheet, since it's never been loaded before.
    117         if (!m_sheet && m_disabledState == EnabledViaScript)
    118             process();
    119         else
    120             document()->styleSelectorChanged(DeferRecalcStyle); // Update the style selector.
    121     }
     88void HTMLLinkElement::setDisabled(bool disabled)
     89{
     90    if (!m_sheet)
     91        return;
     92
     93    bool wasDisabled = m_sheet->disabled();
     94    if (wasDisabled == disabled)
     95        return;
     96
     97    m_sheet->setDisabled(disabled);
     98    m_isEnabledViaScript = !disabled;
     99
     100    // If we change the disabled state while the sheet is still loading, then we have to
     101    // perform three checks:
     102    if (isLoading()) {
     103        // Check #1: The sheet becomes disabled while loading.
     104        if (disabled)
     105            removePendingSheet();
     106
     107        // Check #2: An alternate sheet becomes enabled while it is still loading.
     108        if (m_relAttribute.m_isAlternate && !disabled)
     109            addPendingSheet(Blocking);
     110
     111        // Check #3: A main sheet becomes enabled while it was still loading and
     112        // after it was disabled via script. It takes really terrible code to make this
     113        // happen (a double toggle for no reason essentially). This happens on
     114        // virtualplastic.net, which manages to do about 12 enable/disables on only 3
     115        // sheets. :)
     116        if (!m_relAttribute.m_isAlternate && !disabled && wasDisabled)
     117            addPendingSheet(Blocking);
     118
     119        // If the sheet is already loading just bail.
     120        return;
     121    }
     122
     123    if (!disabled)
     124        process();
    122125}
    123126
    … …  
    141144        m_media = attr->value().string().lower();
    142145        process();
    143     } else if (attr->name() == disabledAttr)
    144         setDisabledState(!attr->isNull());
    145     else if (attr->name() == onbeforeloadAttr)
     146    } else if (attr->name() == onbeforeloadAttr)
    146147        setAttributeEventListener(eventNames().beforeloadEvent, createAttributeEventListener(this, attr));
    147148#if ENABLE(LINK_PREFETCH)
    … …  
    280281    bool acceptIfTypeContainsTextCSS = document()->page() && document()->page()->settings() && document()->page()->settings()->treatsAnyTextCSSLinkAsStylesheet();
    281282   
    282     if (m_disabledState != Disabled && (m_relAttribute.m_isStyleSheet || (acceptIfTypeContainsTextCSS && type.contains("text/css")))
     283    if (!disabled() && (m_relAttribute.m_isStyleSheet || (acceptIfTypeContainsTextCSS && type.contains("text/css")))
    283284        && document()->frame() && m_url.isValid()) {
    284285       
    … …  
    555556}
    556557
    557 void HTMLLinkElement::setDisabled(bool disabled)
    558 {
    559     if (!m_sheet)
    560         return;
    561     m_sheet->setDisabled(disabled);
    562 }
    563 
    564 }
     558}

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

    r87867 r88479  
    7676    StyleSheet* sheet() const;
    7777
     78    // FIXME: This should be remaned isStyleSheetLoading as this is only used for stylesheets.
    7879    bool isLoading() const;
    79 
    80     bool isDisabled() const { return m_disabledState == Disabled; }
    81     bool isEnabledViaScript() const { return m_disabledState == EnabledViaScript; }
     80    bool isEnabledViaScript() const { return m_isEnabledViaScript; }
    8281    bool disabled() const;
    8382    void setDisabled(bool);
    … …  
    104103    virtual void startLoadingDynamicSheet();
    105104
    106     bool isAlternate() const { return m_disabledState == Unset && m_relAttribute.m_isAlternate; }
     105    bool isAlternate() const { return m_relAttribute.m_isAlternate; }
    107106   
    108     void setDisabledState(bool _disabled);
    109 
    110107    virtual bool isURLAttribute(Attribute*) const;
    111108
    … …  
    125122    HTMLLinkElement(const QualifiedName&, Document*, bool createdByParser);
    126123
    127     enum DisabledState {
    128         Unset,
    129         EnabledViaScript,
    130         Disabled
    131     };
    132 
    133124    CachedResourceHandle<CachedCSSStyleSheet> m_cachedSheet;
    134125    RefPtr<CSSStyleSheet> m_sheet;
    … …  
    140131    String m_type;
    141132    String m_media;
    142     DisabledState m_disabledState;
    143133    RelAttribute m_relAttribute;
    144134    bool m_loading;
     135    bool m_isEnabledViaScript;
    145136    bool m_createdByParser;
    146137    bool m_isInShadowTree;
Note: See TracChangeset for help on using the changeset viewer.