[Webkit-unassigned] [Bug 36623] Relative URLs don't work for notifications in Chromium

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 9 17:53:05 PDT 2010


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





--- Comment #4 from John Gregg <johnnyg at google.com>  2010-04-09 17:53:03 PST ---
(From update of attachment 53024)
> diff --git a/LayoutTests/fast/notifications/notifications-with-permission.html-disabled b/LayoutTests/fast/notifications/notifications-with-permission.html-disabled
> index a22717b..7ba887f 100644
> --- a/LayoutTests/fast/notifications/notifications-with-permission.html-disabled
> +++ b/LayoutTests/fast/notifications/notifications-with-permission.html-disabled
> @@ -6,7 +6,7 @@
>          {
>              document.getElementById("result").innerHTML += message + "<br>";
>          }
> -        
> +
>          function runTests()
>          {
>              if (window.layoutTestController) {
> @@ -17,10 +17,13 @@
>              if (!window.webkitNotifications) {
>                  log("FAIL: No webkitNotifications interface!");
>              }
> -            
> +
>              var N = window.webkitNotifications.createHTMLNotification("http://localhost/my_notification.html");
>              N.show();
> -            
> +
> +            var N = window.webkitNotifications.createHTMLNotification("resources/notification.html");
> +            N.show();

Even though this test is disabled you should change the corresponding
expected.txt file.

> +
>              var M = window.webkitNotifications.createNotification("http://localhost/my_icon.png", "New E-mail", "Meet me tonight at 8!");
>              M.show();
>          }
> @@ -28,8 +31,8 @@
>  </head>
>  <body>
>  <p>Sending notifications with permission...</p>
> -    
> -    
> +
> +
>  <script type="text/javascript">
>  runTests();
>  </script>
> diff --git a/LayoutTests/fast/notifications/resources/notification.html b/LayoutTests/fast/notifications/resources/notification.html
> new file mode 100644
> index 0000000..273f256
> --- /dev/null
> +++ b/LayoutTests/fast/notifications/resources/notification.html
> @@ -0,0 +1 @@
> +Hello there. I am a notification.
> diff --git a/WebCore/notifications/Notification.cpp b/WebCore/notifications/Notification.cpp
> index 0c44db6..182f713 100644
> --- a/WebCore/notifications/Notification.cpp
> +++ b/WebCore/notifications/Notification.cpp
> @@ -42,7 +42,7 @@
>  
>  namespace WebCore {
>  
> -Notification::Notification(const String& url, ScriptExecutionContext* context, ExceptionCode& ec, NotificationPresenter* provider)
> +Notification::Notification(const KURL& url, ScriptExecutionContext* context, ExceptionCode& ec, NotificationPresenter* provider)
>      : ActiveDOMObject(context, this)
>      , m_isHTML(true)
>      , m_isShowing(false)
> @@ -54,11 +54,12 @@ Notification::Notification(const String& url, ScriptExecutionContext* context, E
>          return;
>      }
>  
> -    m_notificationURL = context->completeURL(url);
> -    if (url.isEmpty() || !m_notificationURL.isValid()) {
> +    if (url.isEmpty() || !url.isValid()) {
>          ec = SYNTAX_ERR;
>          return;
>      }
> +
> +    m_notificationURL = url;
>  }
>  
>  Notification::Notification(const NotificationContents& contents, ScriptExecutionContext* context, ExceptionCode& ec, NotificationPresenter* provider)
> @@ -74,9 +75,7 @@ Notification::Notification(const NotificationContents& contents, ScriptExecution
>          return;
>      }
>  
> -    if (!contents.icon().isEmpty())
> -        m_iconURL = context->completeURL(contents.icon());
> -    if (!m_iconURL.isEmpty() && !m_iconURL.isValid()) {
> +    if (!contents.icon().isEmpty() && !contents.icon().isValid()) {
>          ec = SYNTAX_ERR;
>          return;
>      }
> diff --git a/WebCore/notifications/Notification.h b/WebCore/notifications/Notification.h
> index c2ba5d9..ff56edc 100644
> --- a/WebCore/notifications/Notification.h
> +++ b/WebCore/notifications/Notification.h
> @@ -55,7 +55,7 @@ namespace WebCore {
>  
>      class Notification : public RefCounted<Notification>, public ActiveDOMObject, public EventTarget { 
>      public:
> -        static Notification* create(const String& url, ScriptExecutionContext* context, ExceptionCode& ec, NotificationPresenter* provider) { return new Notification(url, context, ec, provider); }
> +        static Notification* create(const KURL& url, ScriptExecutionContext* context, ExceptionCode& ec, NotificationPresenter* provider) { return new Notification(url, context, ec, provider); }
>          static Notification* create(const NotificationContents& contents, ScriptExecutionContext* context, ExceptionCode& ec, NotificationPresenter* provider) { return new Notification(contents, context, ec, provider); }
>          
>          virtual ~Notification();
> @@ -65,7 +65,7 @@ namespace WebCore {
>      
>          bool isHTML() { return m_isHTML; }
>          KURL url() { return m_notificationURL; }
> -        KURL iconURL() { return m_iconURL; }
> +        KURL iconURL() { return m_contents.icon(); }
>          NotificationContents& contents() { return m_contents; }
>  
>          DEFINE_ATTRIBUTE_EVENT_LISTENER(display);
> @@ -80,7 +80,7 @@ namespace WebCore {
>          virtual Notification* toNotification() { return this; }
>  
>      private:
> -        Notification(const String& url, ScriptExecutionContext* context, ExceptionCode& ec, NotificationPresenter* provider);
> +        Notification(const KURL& url, ScriptExecutionContext* context, ExceptionCode& ec, NotificationPresenter* provider);
>          Notification(const NotificationContents& fields, ScriptExecutionContext* context, ExceptionCode& ec, NotificationPresenter* provider);
>  
>          // EventTarget interface
> diff --git a/WebCore/notifications/NotificationCenter.h b/WebCore/notifications/NotificationCenter.h
> index ae3dc02..80ab6b7 100644
> --- a/WebCore/notifications/NotificationCenter.h
> +++ b/WebCore/notifications/NotificationCenter.h
> @@ -55,7 +55,7 @@ namespace WebCore {
>                  ec = INVALID_STATE_ERR;
>                  return 0;
>              }
> -            return Notification::create(KURL(ParsedURLString, URI), context(), ec, presenter());
> +            return Notification::create(m_scriptExecutionContext->completeURL(URI), context(), ec, presenter());
>          }
>  
>          Notification* createNotification(const String& iconURI, const String& title, const String& body, ExceptionCode& ec)
> @@ -64,7 +64,7 @@ namespace WebCore {
>                  ec = INVALID_STATE_ERR;
>                  return 0;
>              }
> -            NotificationContents contents(iconURI, title, body);
> +            NotificationContents contents(m_scriptExecutionContext->completeURL(iconURI), title, body);

This is going to be a problem.  If iconURI the string is empty, we don't want
to resolve it to the current script's URL, which is what will happen (I
recently fixed this bug).  We need to make an empty URL.

>              return Notification::create(contents, context(), ec, presenter());
>          }
>  
> diff --git a/WebCore/notifications/NotificationContents.h b/WebCore/notifications/NotificationContents.h
> index ebdc514..2319980 100644
> --- a/WebCore/notifications/NotificationContents.h
> +++ b/WebCore/notifications/NotificationContents.h
> @@ -38,17 +38,17 @@ namespace WebCore {
>      class NotificationContents { 
>      public:
>          NotificationContents() {}
> -        NotificationContents(const String& iconUrl, const String& title, const String& body) 
> +        NotificationContents(const KURL& iconUrl, const String& title, const String& body) 
>              : m_icon(iconUrl)
>              , m_title(title)
>              , m_body(body) {}
>  
> -        String icon() const { return m_icon; }
> +        KURL icon() const { return m_icon; }
>          String title() const { return m_title; }
>          String body() const { return m_body; }
>  
>      private:
> -        String m_icon;
> +        KURL m_icon;
>          String m_title;
>          String m_body;
>      };
> diff --git a/WebKit/chromium/public/WebNotification.h b/WebKit/chromium/public/WebNotification.h
> index b63dd20..9d64e2a 100644
> --- a/WebKit/chromium/public/WebNotification.h
> +++ b/WebKit/chromium/public/WebNotification.h
> @@ -71,9 +71,6 @@ public:
>      // If HTML, the URL which contains the contents of the notification.
>      WEBKIT_API WebURL url() const;
>  
> -    // If not HTML, the parameters for the icon-title-text notification.
> -    // FIXME: Deprecated; use iconURL() instead.
> -    WEBKIT_API WebString icon() const;
>      WEBKIT_API WebURL iconURL() const;
>      WEBKIT_API WebString title() const;
>      WEBKIT_API WebString body() const;
> diff --git a/WebKit/chromium/src/WebNotification.cpp b/WebKit/chromium/src/WebNotification.cpp
> index 26fd4bc..5200d17 100644
> --- a/WebKit/chromium/src/WebNotification.cpp
> +++ b/WebKit/chromium/src/WebNotification.cpp
> @@ -76,13 +76,6 @@ WebURL WebNotification::url() const
>      return m_private->url();
>  }
>  
> -// FIXME: remove this deprecated function once all callers use iconURL()
> -WebString WebNotification::icon() const
> -{
> -    ASSERT(!isHTML());
> -    return m_private->contents().icon();
> -}
> -
>  WebURL WebNotification::iconURL() const
>  {
>      ASSERT(!isHTML());

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list