[Webkit-unassigned] [Bug 62698] [DOM] Support for battery status event

[bugzilla-daemon at webkit.org]

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





--- Comment #10 from Kihong Kwon <kihong.kwon at samsung.com>  2011-06-16 00:14:37 PST ---
(In reply to comment #8)
> (From update of attachment 97255 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=97255&action=review
> 
> It looks like you based this on the location services stuff.  I'm not sure if that's the best model or not for this design.
> 
I referenced device orientation event for this.
Bacause they are almost same operation (transfer device info. to pages)
I'm not sure this is a best choice also,
but I want to make a coherence of these kind of events)

> We need tests for this.  Even if you use a Mock like Location services does.
> 
> > Source/WebCore/ChangeLog:14
> > +        No new tests. (OOPS!)
> 
> This will prevent using the commit-queue.  Normally this line is replaced by a list of tests or reasons why testing is impossible/impractical.
> 
I'll add a testing source for this, and I fix a changlog also.

> > Source/WebCore/dom/BatteryStatusClient.h:29
> > +class BatteryStatusClient {
> 
> Why should this be a WebKit client as opposed to a WebCore platform interface?  I'm not sure that it matters, but I'm curious why you made the choice to go up through the WebKit layer instead of down through WebCore platform.
> 
Client class is a interface for all platform port.
So I think this interface have to be out of platform.
(This can be made all platform port reference a client class)

> > Source/WebCore/dom/BatteryStatusEvent.cpp:29
> > +BatteryStatusEvent::BatteryStatusEvent()
> 
> Why do we need an event subclass for this event?  Does it carry any interesting payload?  Many other APIs just send a named event w/o any subclass.  Then clients know to go look at some property on the Window object when they get the event.
> 
Battery Status Event have to send a battery status to webpages.
It have to have attributes for battery info. That's why I made this is a Event subclass.

> > Source/WebCore/dom/BatteryStatusEvent.cpp:30
> > +    : Event(eventNames().batterystatusEvent, false, true)
> 
> These false/true are meaningless.  Not your fault, but Event should eventually be changed to take enums instead.
> 
OK. I'will fix after enums are made.

> > Source/WebCore/dom/BatteryStatusEvent.cpp:31
> > +    , m_isPlugged(false), m_level(0)
> 
> Normally we do these one per line.

It's fixed.

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