Changeset 57604 in webkit


Ignore:
Timestamp:
Apr 14, 2010 1:51:41 PM (14 years ago)
Author:
aa@chromium.org
Message:

2010-04-14 Aaron Boodman <​aa@chromium.org>

Reviewed by David Levin.

Support relative URLs for notifications on Chromium. They weren't working previously because WebCore was inserting
the relative URL into a KURL instance, but when KURL is backed by GURL as it is on Chromium, relative URLs are
unsupported. Fixed by resolving the relative URL first.

​https://bugs.webkit.org/show_bug.cgi?id=36623

Adding tests for this is difficult because we don't currently have DRT support for notifications on Mac, only Windows.

  • notifications/Notification.cpp: (WebCore::Notification::Notification): Accept resolved KURL instead of relative string.
  • notifications/Notification.h: (WebCore::Notification::create): Ditto. (WebCore::Notification::iconURL): Return resolved KURL instead of relative string.
  • notifications/NotificationCenter.h: (WebCore::NotificationCenter::createHTMLNotification): Immediately resolve URL instead of passing off relative string. (WebCore::NotificationCenter::createNotification): Ditto.
  • notifications/NotificationContents.h: (WebCore::NotificationContents::NotificationContents): Accept resolved KURL instead of relative string. (WebCore::NotificationContents::icon): Return resolved URL.

2010-04-14 Aaron Boodman <​aa@chromium.org>

Reviewed by David Levin.

Support relative URLs for notifications on Chromium. They weren't working previously because WebCore was inserting
the relative URL into a KURL instance, but when KURL is backed by GURL as it is on Chromium, relative URLs are
unsupported. Fixed by resolving the relative URL first.

​https://bugs.webkit.org/show_bug.cgi?id=36623

Adding tests for this is difficult because we don't currently have DRT support for notifications on Mac, only Windows.

  • public/WebNotification.h: Remove deprecated icon() method.
  • src/WebNotification.cpp: Ditto.

2010-04-14 Aaron Boodman <​aa@chromium.org>

Reviewed by David Levin.

Support relative URLs for notifications on Chromium. They weren't working previously because WebCore was inserting
the relative URL into a KURL instance, but when KURL is backed by GURL as it is on Chromium, relative URLs are
unsupported. Fixed by resolving the relative URL first.

​https://bugs.webkit.org/show_bug.cgi?id=36623

Adding tests for this is difficult because we don't currently have DRT support for notifications on Mac, only Windows.

  • WebCoreSupport/NotificationPresenterClientQt.cpp: (NotificationPresenterClientQt::show): Return type of NotificationContents::iconURL() changed.
Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r57603 r57604  
     12010-04-14  Aaron Boodman  <aa@chromium.org>
     2
     3        Reviewed by David Levin.
     4
     5        Support relative URLs for notifications on Chromium. They weren't working previously because WebCore was inserting
     6        the relative URL into a KURL instance, but when KURL is backed by GURL as it is on Chromium, relative URLs are
     7        unsupported. Fixed by resolving the relative URL first.
     8
     9        https://bugs.webkit.org/show_bug.cgi?id=36623
     10
     11        Adding tests for this is difficult because we don't currently have DRT support for notifications on Mac, only Windows.
     12
     13        * notifications/Notification.cpp:
     14        (WebCore::Notification::Notification): Accept resolved KURL instead of relative string.
     15        * notifications/Notification.h:
     16        (WebCore::Notification::create): Ditto.
     17        (WebCore::Notification::iconURL): Return resolved KURL instead of relative string.
     18        * notifications/NotificationCenter.h:
     19        (WebCore::NotificationCenter::createHTMLNotification): Immediately resolve URL instead of passing off relative string.
     20        (WebCore::NotificationCenter::createNotification): Ditto.
     21        * notifications/NotificationContents.h:
     22        (WebCore::NotificationContents::NotificationContents): Accept resolved KURL instead of relative string.
     23        (WebCore::NotificationContents::icon): Return resolved URL.
     24
    1252010-04-14  Anders Carlsson  <andersca@apple.com>
    226

  • trunk/WebCore/notifications/Notification.cpp

    r57103 r57604  
    4343namespace WebCore {
    4444
    45 Notification::Notification(const String& url, ScriptExecutionContext* context, ExceptionCode& ec, NotificationPresenter* provider)
     45Notification::Notification(const KURL& url, ScriptExecutionContext* context, ExceptionCode& ec, NotificationPresenter* provider)
    4646    : ActiveDOMObject(context, this)
    4747    , m_isHTML(true)
    … …  
    5555    }
    5656
    57     m_notificationURL = context->completeURL(url);
    58     if (url.isEmpty() || !m_notificationURL.isValid()) {
     57    if (url.isEmpty() || !url.isValid()) {
    5958        ec = SYNTAX_ERR;
    6059        return;
    6160    }
     61
     62    m_notificationURL = url;
    6263}
    6364
    … …  
    7576    }
    7677
    77     if (!contents.icon().isEmpty())
    78         m_iconURL = context->completeURL(contents.icon());
    79     if (!m_iconURL.isEmpty() && !m_iconURL.isValid()) {
     78    if (!contents.icon().isEmpty() && !contents.icon().isValid()) {
    8079        ec = SYNTAX_ERR;
    8180        return;

  • trunk/WebCore/notifications/Notification.h

    r56043 r57604  
    5656    class Notification : public RefCounted<Notification>, public ActiveDOMObject, public EventTarget {
    5757    public:
    58         static Notification* create(const String& url, ScriptExecutionContext* context, ExceptionCode& ec, NotificationPresenter* provider) { return new Notification(url, context, ec, provider); }
     58        static Notification* create(const KURL& url, ScriptExecutionContext* context, ExceptionCode& ec, NotificationPresenter* provider) { return new Notification(url, context, ec, provider); }
    5959        static Notification* create(const NotificationContents& contents, ScriptExecutionContext* context, ExceptionCode& ec, NotificationPresenter* provider) { return new Notification(contents, context, ec, provider); }
    6060       
    … …  
    6666        bool isHTML() { return m_isHTML; }
    6767        KURL url() { return m_notificationURL; }
    68         KURL iconURL() { return m_iconURL; }
     68        KURL iconURL() { return m_contents.icon(); }
    6969        NotificationContents& contents() { return m_contents; }
    7070
    … …  
    8181
    8282    private:
    83         Notification(const String& url, ScriptExecutionContext* context, ExceptionCode& ec, NotificationPresenter* provider);
    84         Notification(const NotificationContents& fields, ScriptExecutionContext* context, ExceptionCode& ec, NotificationPresenter* provider);
     83        Notification(const KURL&, ScriptExecutionContext*, ExceptionCode&, NotificationPresenter*);
     84        Notification(const NotificationContents&, ScriptExecutionContext*, ExceptionCode&, NotificationPresenter*);
    8585
    8686        // EventTarget interface
    … …  
    9292        bool m_isHTML;
    9393        KURL m_notificationURL;
    94         KURL m_iconURL;
    9594        NotificationContents m_contents;
    9695

  • trunk/WebCore/notifications/NotificationCenter.h

    r51865 r57604  
    5656                return 0;
    5757            }
    58             return Notification::create(KURL(ParsedURLString, URI), context(), ec, presenter());
     58            if (URI.isEmpty()) {
     59                ec = SYNTAX_ERR;
     60                return 0;
     61            }
     62            return Notification::create(m_scriptExecutionContext->completeURL(URI), context(), ec, presenter());
    5963        }
    6064
    … …  
    6569                return 0;
    6670            }
    67             NotificationContents contents(iconURI, title, body);
     71            NotificationContents contents(iconURI.isEmpty() ? KURL() : m_scriptExecutionContext->completeURL(iconURI), title, body);
    6872            return Notification::create(contents, context(), ec, presenter());
    6973        }

  • trunk/WebCore/notifications/NotificationContents.h

    r47056 r57604  
    3939    public:
    4040        NotificationContents() {}
    41         NotificationContents(const String& iconUrl, const String& title, const String& body)
     41        NotificationContents(const KURL& iconUrl, const String& title, const String& body)
    4242            : m_icon(iconUrl)
    4343            , m_title(title)
    4444            , m_body(body) {}
    4545
    46         String icon() const { return m_icon; }
     46        KURL icon() const { return m_icon; }
    4747        String title() const { return m_title; }
    4848        String body() const { return m_body; }
    4949
    5050    private:
    51         String m_icon;
     51        KURL m_icon;
    5252        String m_title;
    5353        String m_body;

  • trunk/WebKit/chromium/ChangeLog

    r57599 r57604  
     12010-04-14  Aaron Boodman  <aa@chromium.org>
     2
     3        Reviewed by David Levin.
     4
     5        Support relative URLs for notifications on Chromium. They weren't working previously because WebCore was inserting
     6        the relative URL into a KURL instance, but when KURL is backed by GURL as it is on Chromium, relative URLs are
     7        unsupported. Fixed by resolving the relative URL first.
     8
     9        https://bugs.webkit.org/show_bug.cgi?id=36623
     10
     11        Adding tests for this is difficult because we don't currently have DRT support for notifications on Mac, only Windows.
     12
     13        * public/WebNotification.h: Remove deprecated icon() method.
     14        * src/WebNotification.cpp: Ditto.
     15
    1162010-04-14  Jay Civelli  <jcivelli@chromium.org>
    217

  • trunk/WebKit/chromium/public/WebNotification.h

    r56043 r57604  
    7272    WEBKIT_API WebURL url() const;
    7373
    74     // If not HTML, the parameters for the icon-title-text notification.
    75     // FIXME: Deprecated; use iconURL() instead.
    76     WEBKIT_API WebString icon() const;
    7774    WEBKIT_API WebURL iconURL() const;
    7875    WEBKIT_API WebString title() const;

  • trunk/WebKit/chromium/src/WebNotification.cpp

    r56043 r57604  
    7777}
    7878
    79 // FIXME: remove this deprecated function once all callers use iconURL()
    80 WebString WebNotification::icon() const
    81 {
    82     ASSERT(!isHTML());
    83     return m_private->contents().icon();
    84 }
    85 
    8679WebURL WebNotification::iconURL() const
    8780{

  • trunk/WebKit/qt/ChangeLog

    r57535 r57604  
     12010-04-14  Aaron Boodman  <aa@chromium.org>
     2
     3        Reviewed by David Levin.
     4
     5        Support relative URLs for notifications on Chromium. They weren't working previously because WebCore was inserting
     6        the relative URL into a KURL instance, but when KURL is backed by GURL as it is on Chromium, relative URLs are
     7        unsupported. Fixed by resolving the relative URL first.
     8
     9        https://bugs.webkit.org/show_bug.cgi?id=36623
     10
     11        Adding tests for this is difficult because we don't currently have DRT support for notifications on Mac, only Windows.
     12
     13        * WebCoreSupport/NotificationPresenterClientQt.cpp:
     14        (NotificationPresenterClientQt::show): Return type of NotificationContents::iconURL() changed.
     15
    1162010-04-13  Timothy Hatcher  <timothy@apple.com>
    217

  • trunk/WebKit/qt/WebCoreSupport/NotificationPresenterClientQt.cpp

    r57408 r57604  
    6464        else {
    6565            printf("DESKTOP NOTIFICATION: icon %s, title %s, text %s\n",
    66                 QString(notification->contents().icon()).toUtf8().constData(), QString(notification->contents().title()).toUtf8().constData(),
     66                QString(notification->contents().icon().string()).toUtf8().constData(), QString(notification->contents().title()).toUtf8().constData(),
    6767                QString(notification->contents().body()).toUtf8().constData());
    6868        }
Note: See TracChangeset for help on using the changeset viewer.