Changeset 64085 in webkit


Ignore:
Timestamp:
Jul 26, 2010 3:58:33 PM (14 years ago)
Author:
Darin Adler
Message:

2010-07-26 Darin Adler <Darin Adler>

Reviewed by Eric Carlson.

Don't handle empty URL attributes specially in reflection, even in cases such as <img src="">
​https://bugs.webkit.org/show_bug.cgi?id=42087

  • html/HTMLMediaElement.cpp: (WebCore::HTMLMediaElement::insertedIntoDocument): Check if the src attribute is empty by calling getAttribute instead of calling the src function. This keeps the behavior correct with the reflection change, and has a side benefit of being more efficient as well since there is no reason to resolve the URL. (WebCore::HTMLMediaElement::loadResource): Call getNonEmptyURLAttribute directly here instead of using the poster function. (WebCore::HTMLMediaElement::getPluginProxyParams): Ditto.
  • html/HTMLMediaElement.h: Removed the poster virtual function. The code that gets the poster attribute is already video-specific and uses generic attribute fetching code, so we no longer need a virtual function. Removed the src function. Since this would give a different result from the JavaScript reflected src attribute, it's safer not to have it, and it was easy to convert all the callers.
  • html/HTMLMediaElement.idl: Removed the NonEmpty extended attribute from the src attribute.
  • html/HTMLSourceElement.cpp: Removed the src function for the same reason as above.
  • html/HTMLSourceElement.h: Ditto.
  • html/HTMLSourceElement.idl: Removed the NonEmpty extended attribute from the src attribute.
  • html/HTMLVideoElement.cpp: (WebCore::HTMLVideoElement::parseMappedAttribute): Removed code to update m_posterURL, which is no longer needed. Changed call site that used the poster() function to instead use getNonEmptyURLAttribute. (WebCore::HTMLVideoElement::updatePosterImage): Check if the poster attribute is empty by calling getAttribute instead of calling the poster function. This keeps the behavior correct with the reflection change, and has a side benefit of being more efficient as well since there is no reason to resolve the URL.
  • html/HTMLVideoElement.h: Removed the poster function for the same reason we removed the src function above. Also removed now-unneeded m_posterURL data member.
  • html/HTMLVideoElement.idl: Removed the NonEmpty extended attribute from the poster attribute.

2010-07-26 Darin Adler <Darin Adler>

Reviewed by Eric Carlson.

Don’t handle empty URL attributes specially in reflection, even in cases such as <img src="">
​https://bugs.webkit.org/show_bug.cgi?id=42087

  • fast/dom/URL-attribute-reflection-expected.txt: Updated.
  • fast/dom/script-tests/URL-attribute-reflection.js: Updated test expectations so that even for URLs where "" is invalid, we reflect the empty string resolved to the base URL.
  • media/video-poster-expected.txt: Updated.
  • media/video-poster.html: Updated to expect poster="" to reflect as the base URL.
  • media/video-source-error-no-candidate-expected.txt: Updated.
  • media/video-src-none-expected.txt: Updated.
  • media/video-src-none.html: Updated to expect src="" to reflect as the base URL.
Location:
trunk
Files:
18 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r64083 r64085  
     12010-07-26  Darin Adler  <darin@apple.com>
     2
     3        Reviewed by Eric Carlson.
     4
     5        Don’t handle empty URL attributes specially in reflection, even in cases such as <img src="">
     6        https://bugs.webkit.org/show_bug.cgi?id=42087
     7
     8        * fast/dom/URL-attribute-reflection-expected.txt: Updated.
     9        * fast/dom/script-tests/URL-attribute-reflection.js: Updated test expectations so that even
     10        for URLs where "" is invalid, we reflect the empty string resolved to the base URL.
     11
     12        * media/video-poster-expected.txt: Updated.
     13        * media/video-poster.html: Updated to expect poster="" to reflect as the base URL.
     14        * media/video-source-error-no-candidate-expected.txt: Updated.
     15        * media/video-src-none-expected.txt: Updated.
     16        * media/video-src-none.html: Updated to expect src="" to reflect as the base URL.
     17
    1182010-07-26  Ryosuke Niwa  <rniwa@webkit.org>
    219

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

    r63119 r64085  
    1111FAIL testURLReflection('cite', 'ins') should be URL. Was non-URL.
    1212FAIL testURLReflection('cite', 'q') should be URL. Was non-URL.
    13 FAIL testURLReflection('data', 'object') should be non-empty URL. Was URL.
     13PASS testURLReflection('data', 'object') is 'URL'
    1414FAIL testURLReflection('formaction', 'button') should be URL. Was none.
    1515FAIL testURLReflection('formaction', 'input') should be URL. Was none.
    1616PASS testURLReflection('href', 'a') is 'URL'
    1717PASS testURLReflection('href', 'area') is 'URL'
    18 FAIL testURLReflection('href', 'link') should be non-empty URL. Was URL.
     18PASS testURLReflection('href', 'link') is 'URL'
    1919FAIL testURLReflection('href', 'base') should be URL. Was non-URL.
    20 FAIL testURLReflection('icon', 'command') should be non-empty URL. Was none.
    21 FAIL testURLReflection('manifest', 'html') should be non-empty URL. Was none.
    22 PASS testURLReflection('poster', 'video') is 'non-empty URL'
    23 PASS testURLReflection('src', 'audio') is 'non-empty URL'
    24 FAIL testURLReflection('src', 'embed') should be non-empty URL. Was non-URL.
    25 FAIL testURLReflection('src', 'iframe') should be non-empty URL. Was non-URL.
    26 FAIL testURLReflection('src', 'img') should be non-empty URL. Was URL.
    27 FAIL testURLReflection('src', 'input') should be non-empty URL. Was URL.
    28 FAIL testURLReflection('src', 'script') should be non-empty URL. Was URL.
    29 PASS testURLReflection('src', 'source') is 'non-empty URL'
    30 PASS testURLReflection('src', 'video') is 'non-empty URL'
    31 FAIL testURLReflection('longDesc', 'img') should be non-empty URL. Was URL.
    32 FAIL testURLReflection('lowsrc', 'img') should be non-empty URL. Was URL.
     20FAIL testURLReflection('icon', 'command') should be URL. Was none.
     21FAIL testURLReflection('manifest', 'html') should be URL. Was none.
     22PASS testURLReflection('poster', 'video') is 'URL'
     23PASS testURLReflection('src', 'audio') is 'URL'
     24FAIL testURLReflection('src', 'embed') should be URL. Was non-URL.
     25FAIL testURLReflection('src', 'iframe') should be URL. Was non-URL.
     26PASS testURLReflection('src', 'img') is 'URL'
     27PASS testURLReflection('src', 'input') is 'URL'
     28PASS testURLReflection('src', 'script') is 'URL'
     29PASS testURLReflection('src', 'source') is 'URL'
     30PASS testURLReflection('src', 'video') is 'URL'
     31PASS testURLReflection('longDesc', 'img') is 'URL'
     32PASS testURLReflection('lowsrc', 'img') is 'URL'
    3333PASS successfullyParsed is true
    3434

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

    r63057 r64085  
    3737
    3838// The following list comes from the HTML5 document’s attributes index.
    39 // The expected results are based on what the table there says.
     39// These are the URL attributes from that list.
    4040
    4141shouldBe("testURLReflection('action', 'form')", "'URL'");
    … …  
    4444shouldBe("testURLReflection('cite', 'ins')", "'URL'");
    4545shouldBe("testURLReflection('cite', 'q')", "'URL'");
    46 shouldBe("testURLReflection('data', 'object')", "'non-empty URL'");
     46shouldBe("testURLReflection('data', 'object')", "'URL'");
    4747shouldBe("testURLReflection('formaction', 'button')", "'URL'");
    4848shouldBe("testURLReflection('formaction', 'input')", "'URL'");
    4949shouldBe("testURLReflection('href', 'a')", "'URL'");
    5050shouldBe("testURLReflection('href', 'area')", "'URL'");
    51 shouldBe("testURLReflection('href', 'link')", "'non-empty URL'");
     51shouldBe("testURLReflection('href', 'link')", "'URL'");
    5252shouldBe("testURLReflection('href', 'base')", "'URL'");
    53 shouldBe("testURLReflection('icon', 'command')", "'non-empty URL'");
    54 shouldBe("testURLReflection('manifest', 'html')", "'non-empty URL'");
    55 shouldBe("testURLReflection('poster', 'video')", "'non-empty URL'");
    56 shouldBe("testURLReflection('src', 'audio')", "'non-empty URL'");
    57 shouldBe("testURLReflection('src', 'embed')", "'non-empty URL'");
    58 shouldBe("testURLReflection('src', 'iframe')", "'non-empty URL'");
    59 shouldBe("testURLReflection('src', 'img')", "'non-empty URL'");
    60 shouldBe("testURLReflection('src', 'input')", "'non-empty URL'");
    61 shouldBe("testURLReflection('src', 'script')", "'non-empty URL'");
    62 shouldBe("testURLReflection('src', 'source')", "'non-empty URL'");
    63 shouldBe("testURLReflection('src', 'video')", "'non-empty URL'");
     53shouldBe("testURLReflection('icon', 'command')", "'URL'");
     54shouldBe("testURLReflection('manifest', 'html')", "'URL'");
     55shouldBe("testURLReflection('poster', 'video')", "'URL'");
     56shouldBe("testURLReflection('src', 'audio')", "'URL'");
     57shouldBe("testURLReflection('src', 'embed')", "'URL'");
     58shouldBe("testURLReflection('src', 'iframe')", "'URL'");
     59shouldBe("testURLReflection('src', 'img')", "'URL'");
     60shouldBe("testURLReflection('src', 'input')", "'URL'");
     61shouldBe("testURLReflection('src', 'script')", "'URL'");
     62shouldBe("testURLReflection('src', 'source')", "'URL'");
     63shouldBe("testURLReflection('src', 'video')", "'URL'");
    6464
    6565// Other reflected URL attributes.
    6666
    67 shouldBe("testURLReflection('longDesc', 'img')", "'non-empty URL'");
    68 shouldBe("testURLReflection('lowsrc', 'img')", "'non-empty URL'");
     67shouldBe("testURLReflection('longDesc', 'img')", "'URL'");
     68shouldBe("testURLReflection('lowsrc', 'img')", "'URL'");
    6969
    7070var successfullyParsed = true;

  • trunk/LayoutTests/media/video-poster-expected.txt

    r63119 r64085  
    66Set 'poster' to ''.
    77EXPECTED (video.getAttribute('poster') == '') OK
    8 EXPECTED (relativeURL(video.poster) == '') OK
     8EXPECTED (relativeURL(video.poster) == 'video-poster.html') OK
    99
    1010Set 'poster' to 'content/abe.png'.

  • trunk/LayoutTests/media/video-poster.html

    r63119 r64085  
    1919                consoleWrite("<br><i>Set 'poster' to ''.<" + "/i>");
    2020                video.setAttribute('poster', '');
    21                 testPoster('');
     21                testExpected("video.getAttribute('poster')", '');
     22                testExpected("relativeURL(video.poster)", 'video-poster.html');
    2223
    2324                consoleWrite("<br><i>Set 'poster' to 'content/abe.png'.<" + "/i>");

  • trunk/LayoutTests/media/video-source-error-no-candidate-expected.txt

    r63119 r64085  
    1111EXPECTED (video.error == 'null') OK
    1212
    13 EVENT(error) from <source id='empty-src' src=''> OK
     13EVENT(error) from <source id='empty-src' src='video-source-error-no-candidate.html'> OK
    1414EXPECTED (video.error == 'null') OK
    1515

  • trunk/LayoutTests/media/video-src-none-expected.txt

    r63684 r64085  
    99EXPECTED (videos[1].error == 'null') OK
    1010EXPECTED (videos[1].networkState == '0') OK
    11 EXPECTED (videos[1].src == '') OK
     11EXPECTED (relativeURL(videos[1].src) == 'video-src-none.html') OK
    1212
    1313Calling load() with empty 'src' should fire 'error' event, set network state to NETWORK_NO_SOURCE, and set error to MEDIA_ERR_SRC_NOT_SUPPORTED.

  • trunk/LayoutTests/media/video-src-none.html

    r63684 r64085  
    4343                testExpected("videos[1].error", null);
    4444                testExpected("videos[1].networkState", HTMLMediaElement.NETWORK_EMPTY);
    45                 testExpected("videos[1].src", "");
     45                testExpected("relativeURL(videos[1].src)", "video-src-none.html");
    4646
    4747                consoleWrite("<br><" + "i>Calling load() with empty 'src' should fire 'error' event, set network state to NETWORK_NO_SOURCE, and set error to MEDIA_ERR_SRC_NOT_SUPPORTED.<" + "/i>");

  • trunk/WebCore/ChangeLog

    r64084 r64085  
     12010-07-26  Darin Adler  <darin@apple.com>
     2
     3        Reviewed by Eric Carlson.
     4
     5        Don't handle empty URL attributes specially in reflection, even in cases such as <img src="">
     6        https://bugs.webkit.org/show_bug.cgi?id=42087
     7
     8        * html/HTMLMediaElement.cpp:
     9        (WebCore::HTMLMediaElement::insertedIntoDocument): Check if the src attribute is empty
     10        by calling getAttribute instead of calling the src function. This keeps the behavior
     11        correct with the reflection change, and has a side benefit of being more efficient as
     12        well since there is no reason to resolve the URL.
     13        (WebCore::HTMLMediaElement::loadResource): Call getNonEmptyURLAttribute directly here
     14        instead of using the poster function.
     15        (WebCore::HTMLMediaElement::getPluginProxyParams): Ditto.
     16
     17        * html/HTMLMediaElement.h: Removed the poster virtual function. The code that gets the
     18        poster attribute is already video-specific and uses generic attribute fetching code, so
     19        we no longer need a virtual function. Removed the src function. Since this would give
     20        a different result from the JavaScript reflected src attribute, it's safer not to have
     21        it, and it was easy to convert all the callers.
     22
     23        * html/HTMLMediaElement.idl: Removed the NonEmpty extended attribute from the src
     24        attribute.
     25
     26        * html/HTMLSourceElement.cpp: Removed the src function for the same reason as above.
     27        * html/HTMLSourceElement.h: Ditto.
     28
     29        * html/HTMLSourceElement.idl: Removed the NonEmpty extended attribute from the src
     30        attribute.
     31
     32        * html/HTMLVideoElement.cpp:
     33        (WebCore::HTMLVideoElement::parseMappedAttribute): Removed code to update m_posterURL,
     34        which is no longer needed. Changed call site that used the poster() function to instead
     35        use getNonEmptyURLAttribute.
     36        (WebCore::HTMLVideoElement::updatePosterImage): Check if the poster attribute is empty
     37        by calling getAttribute instead of calling the poster function. This keeps the behavior
     38        correct with the reflection change, and has a side benefit of being more efficient as
     39        well since there is no reason to resolve the URL.
     40
     41        * html/HTMLVideoElement.h: Removed the poster function for the same reason we removed
     42        the src function above. Also removed now-unneeded m_posterURL data member.
     43
     44        * html/HTMLVideoElement.idl: Removed the NonEmpty extended attribute from the poster
     45        attribute.
     46
    1472010-07-26  Chris Fleizach  <cfleizach@apple.com>
    248

  • trunk/WebCore/html/HTMLMediaElement.cpp

    r63844 r64085  
    305305{
    306306    HTMLElement::insertedIntoDocument();
    307     if (!src().isEmpty() && m_networkState == NETWORK_EMPTY)
     307    if (!getAttribute(srcAttr).isEmpty() && m_networkState == NETWORK_EMPTY)
    308308        scheduleLoad();
    309309}
    … …  
    437437{
    438438    return m_error;
    439 }
    440 
    441 KURL HTMLMediaElement::src() const
    442 {
    443     return getNonEmptyURLAttribute(srcAttr);
    444439}
    445440
    … …  
    672667
    673668    if (isVideo() && m_player->canLoadPoster()) {
    674         KURL posterUrl = poster();
    675         if (!posterUrl.isEmpty())
    676             m_player->setPoster(posterUrl);
     669        KURL posterURL = getNonEmptyURLAttribute(posterAttr);
     670        if (!posterURL.isEmpty())
     671            m_player->setPoster(posterURL);
    677672    }
    678673
    … …  
    19631958
    19641959    if (isVideo()) {
    1965         String poster = poster();
    1966         if (!poster.isEmpty() && loader) {
    1967             KURL posterURL = loader->completeURL(poster);
    1968             if (posterURL.isValid() && loader->willLoadMediaElementURL(posterURL)) {
    1969                 names.append("_media_element_poster_");
    1970                 values.append(posterURL.string());
    1971             }
     1960        KURL posterURL = getNonEmptyURLAttribute(posterAttr);
     1961        if (!posterURL.isEmpty() && loader && loader->willLoadMediaElementURL(posterURL)) {
     1962            names.append("_media_element_poster_");
     1963            values.append(posterURL.string());
    19721964        }
    19731965    }

  • trunk/WebCore/html/HTMLMediaElement.h

    r63684 r64085  
    6565    // Eventually overloaded in HTMLVideoElement
    6666    virtual bool supportsFullscreen() const { return false; };
    67     virtual const KURL poster() const { return KURL(); }
    6867
    6968    virtual bool supportsSave() const;
    … …  
    8584
    8685// network state
    87     KURL src() const;
    8886    void setSrc(const String&);
    8987    String currentSrc() const;

  • trunk/WebCore/html/HTMLMediaElement.idl

    r63684 r64085  
    3131
    3232    // network state
    33     attribute [Reflect,NonEmpty,URL] DOMString src;
     33    attribute [Reflect,URL] DOMString src;
    3434    readonly attribute DOMString currentSrc;
    3535   

  • trunk/WebCore/html/HTMLSourceElement.cpp

    r63119 r64085  
    6363}
    6464
    65 KURL HTMLSourceElement::src() const
    66 {
    67     return getNonEmptyURLAttribute(srcAttr);
    68 }
    69 
    7065void HTMLSourceElement::setSrc(const String& url)
    7166{

  • trunk/WebCore/html/HTMLSourceElement.h

    r63119 r64085  
    3838    static PassRefPtr<HTMLSourceElement> create(const QualifiedName&, Document*);
    3939
    40     KURL src() const;
    4140    String media() const;
    4241    String type() const;

  • trunk/WebCore/html/HTMLSourceElement.idl

    r63119 r64085  
    2626module html {
    2727interface [Conditional=VIDEO] HTMLSourceElement : HTMLElement {
    28     attribute [Reflect,NonEmpty,URL] DOMString src;
     28    attribute [Reflect,URL] DOMString src;
    2929    attribute DOMString type;
    3030    attribute DOMString media;

  • trunk/WebCore/html/HTMLVideoElement.cpp

    r63119 r64085  
    102102
    103103    if (attrName == posterAttr) {
    104         m_posterURL = getNonEmptyURLAttribute(posterAttr);
    105104        updatePosterImage();
    106105        if (m_shouldDisplayPosterImage) {
    … …  
    111110#else
    112111            if (player())
    113                 player()->setPoster(poster());
     112                player()->setPoster(getNonEmptyURLAttribute(posterAttr));
    114113#endif
    115114        }
    … …  
    180179#endif
    181180
    182     m_shouldDisplayPosterImage = !poster().isEmpty() && !hasAvailableVideoFrame();
     181    m_shouldDisplayPosterImage = !getAttribute(posterAttr).isEmpty() && !hasAvailableVideoFrame();
    183182
    184183#if !ENABLE(PLUGIN_PROXY_FOR_VIDEO)

  • trunk/WebCore/html/HTMLVideoElement.h

    r61959 r64085  
    4545    unsigned videoHeight() const;
    4646   
    47     virtual const KURL poster() const { return m_posterURL; }
    48 
    4947    // Fullscreen
    5048    void webkitEnterFullscreen(bool isUserGesture, ExceptionCode&);
    … …  
    8583
    8684    OwnPtr<HTMLImageLoader> m_imageLoader;
    87     KURL m_posterURL;
    8885    bool m_shouldDisplayPosterImage;
    8986};

  • trunk/WebCore/html/HTMLVideoElement.idl

    r63119 r64085  
    3333        readonly attribute unsigned long videoWidth;
    3434        readonly attribute unsigned long videoHeight;
    35         attribute [Reflect,NonEmpty,URL] DOMString poster;
     35        attribute [Reflect,URL] DOMString poster;
    3636
    3737        readonly attribute boolean webkitSupportsFullscreen;
Note: See TracChangeset for help on using the changeset viewer.