Attachment 339487 Details for Bug 185278 – Patch

[patch] Patch

bug-185278-20180503195852.patch (text/plain), 3.96 KB, created by Chris Nardi on 2018-05-03 16:58:52 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Nardi

Created: 2018-05-03 16:58:52 PDT

Size: 3.96 KB

patch

obsolete

>Subversion Revision: 231306
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index bee00c2c6c681b3d725ec9f576ded3cec8bd05ff..87d9de1b0fa643c48d3e0130ace2c46854122134 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2018-05-03  Chris Nardi  <cnardi@chromium.org>
>+
>+        Remove dead exception in MediaList.appendMedium
>+        https://bugs.webkit.org/show_bug.cgi?id=185278
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        MediaList.appendMedium was able to throw an exception, but MediaQuerySet::add() always
>+        returned true, making it impossible for that exception to be thrown. This matched the
>+        spec, as |appendMedium| is not specified to throw an exception. Remove the dead code
>+        surrounding the exception, and make MediaQuerySet::add() return false if the medium is
>+        not added.
>+
>+        No new/modified tests as there should be no functional changes.
>+
>+        * css/MediaList.cpp:
>+        (WebCore::MediaQuerySet::add):
>+        (WebCore::MediaList::appendMedium):
>+        * css/MediaList.h:
>+        * css/MediaList.idl:
>+
> 2018-05-03  Ryan Haddad  <ryanhaddad@apple.com>
> 
>         Unreviewed, rolling out r231253.
>diff --git a/Source/WebCore/css/MediaList.cpp b/Source/WebCore/css/MediaList.cpp
>index 45750af97567ac2ab6b0344c538e5cfd8aaad54c..6cc5fcf55642529134510d4ab444b604563dbc7b 100644
>--- a/Source/WebCore/css/MediaList.cpp
>+++ b/Source/WebCore/css/MediaList.cpp
>@@ -96,13 +96,13 @@ bool MediaQuerySet::add(const String& queryString)
>     
>     // Only continue if exactly one media query is found, as described above.
>     if (result->m_queries.size() != 1)
>-        return true;
>+        return false;
>     
>     // If comparing with any of the media queries in the collection of media
>     // queries returns true terminate these steps.
>     for (size_t i = 0; i < m_queries.size(); ++i) {
>         if (m_queries[i] == result->m_queries[0])
>-            return true;
>+            return false;
>     }
>     
>     m_queries.append(result->m_queries[0]);
>@@ -204,18 +204,15 @@ ExceptionOr<void> MediaList::deleteMedium(const String& medium)
>     return { };
> }
> 
>-ExceptionOr<void> MediaList::appendMedium(const String& medium)
>+void MediaList::appendMedium(const String& medium)
> {
>     CSSStyleSheet::RuleMutationScope mutationScope(m_parentRule);
> 
>-    bool success = m_mediaQueries->add(medium);
>-    if (!success) {
>-        // FIXME: Should this really be InvalidCharacterError?
>-        return Exception { InvalidCharacterError };
>-    }
>+    bool added = m_mediaQueries->add(medium);
>+    if (!added)
>+        return;
>     if (m_parentStyleSheet)
>         m_parentStyleSheet->didMutate();
>-    return { };
> }
> 
> void MediaList::reattach(MediaQuerySet* mediaQueries)
>diff --git a/Source/WebCore/css/MediaList.h b/Source/WebCore/css/MediaList.h
>index 07896e02692cf1fe872da2138f34257b52ed853a..4cbd401123924f34e320b0d7d7913f07a1c325b2 100644
>--- a/Source/WebCore/css/MediaList.h
>+++ b/Source/WebCore/css/MediaList.h
>@@ -90,7 +90,7 @@ public:
>     unsigned length() const { return m_mediaQueries->queryVector().size(); }
>     WEBCORE_EXPORT String item(unsigned index) const;
>     WEBCORE_EXPORT ExceptionOr<void> deleteMedium(const String& oldMedium);
>-    WEBCORE_EXPORT ExceptionOr<void> appendMedium(const String& newMedium);
>+    WEBCORE_EXPORT void appendMedium(const String& newMedium);
> 
>     String mediaText() const { return m_mediaQueries->mediaText(); }
>     WEBCORE_EXPORT ExceptionOr<void> setMediaText(const String&);
>diff --git a/Source/WebCore/css/MediaList.idl b/Source/WebCore/css/MediaList.idl
>index cf6ac4e83977f8faafb586476cec5cb5de41f29f..21dd7abb590d4051fa828a58ecdd0de399d1bb29 100644
>--- a/Source/WebCore/css/MediaList.idl
>+++ b/Source/WebCore/css/MediaList.idl
>@@ -35,5 +35,5 @@
>     getter DOMString? item(unsigned long index);
> 
>     [MayThrowException] void deleteMedium(DOMString oldMedium);
>-    [MayThrowException] void appendMedium(DOMString newMedium);
>+    void appendMedium(DOMString newMedium);
> };

Actions: View | Formatted Diff | Diff

Attachments on bug 185278: 339487 | 339493 | 339496 | 339557 | 339564 | 340944