[Webkit-unassigned] [Bug 73194] [BlackBerry] Add notification support for the BlackBerry port

[bugzilla-daemon at webkit.org]

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





--- Comment #6 from Robin Qiu <robin.qiu at torchmobile.com.cn>  2011-11-29 23:56:02 PST ---
(From update of attachment 117108)
View in context: https://bugs.webkit.org/attachment.cgi?id=117108&action=review

>> Source/WebKit/blackberry/WebCoreSupport/NotificationPresenterImpl.cpp:72
>> +        // FIXME: strip to one line
> 
> Nit: strip => Strip
> 
> Also, comments should be full sentences that end in a period.
> 
> I'm not entirely clear what this comment means. I take it that either notification->contents().title() or notification->contents().body() may contain some kind of line delimiter character(s) (e.g. new line characters) and that we want to remove such characters such that the message is ultimately displayed as one line of text.

Yes. That's what I mean. :)

>> Source/WebKit/blackberry/WebCoreSupport/NotificationPresenterImpl.h:30
>> +#include "StringHash.h"
> 
> Do we need to include this file here? It should be sufficient to only include this file in NotificationPresenterImpl.cpp (which we already do).

Yes. StringHash.h is needed in HashSet<WTF::String> instantiation.

>> Source/WebKit/blackberry/WebCoreSupport/NotificationPresenterImpl.h:58
>> +    // Cancel all outstanding requests for the ScriptExecutionContext
> 
> This comment seems inane given the name of this function. I suggest removing it.

This comment is copied from its base class, I think I'd better remain it.

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