Bug 63376 - [DOM] Core part patch for supporting battery status event
: [DOM] Core part patch for supporting battery status event
Status: RESOLVED INVALID
: WebKit
HTML DOM
: 528+ (Nightly build)
: All All
: P3 Normal
Assigned To:
: http://dev.w3.org/2009/dap/system-inf...
:
:
: 62698
  Show dependency treegraph
 
Reported: 2011-06-24 21:53 PST by
Modified: 2012-04-05 00:15 PST (History)


Attachments
Add core patch files for the battery status event (11.58 KB, patch)
2011-06-24 22:26 PST, Kihong Kwon
gyuyoung.kim: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Add core patch files for the battery status event. (10.58 KB, patch)
2011-06-24 22:57 PST, Kihong Kwon
no flags Review Patch | Details | Formatted Diff | Diff
Add core patch files for the battery status event (10.08 KB, patch)
2011-06-26 18:12 PST, Kihong Kwon
leandro: review-
leandro: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Modify description in the ChangeLog (10.25 KB, patch)
2011-06-27 18:36 PST, Kihong Kwon
leandro: review-
leandro: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Modify ChangeLog's comments (10.16 KB, patch)
2011-07-01 02:15 PST, Kihong Kwon
no flags Review Patch | Details | Formatted Diff | Diff
Modify comment of ChangLog (10.12 KB, patch)
2011-07-01 03:49 PST, Kihong Kwon
no flags Review Patch | Details | Formatted Diff | Diff
Add events for the revised spec. (11.80 KB, patch)
2011-07-06 23:43 PST, Kihong Kwon
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-06-24 21:53:59 PST
This patch is a core part patch for battery status event(related bug 62698)
have to be patched after bug 62698
------- Comment #1 From 2011-06-24 22:26:30 PST -------
Created an attachment (id=98579) [details]
Add core patch files for the battery status event
------- Comment #2 From 2011-06-24 22:52:22 PST -------
(From update of attachment 98579 [details])
Attachment 98579 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/8934704
------- Comment #3 From 2011-06-24 22:57:27 PST -------
Created an attachment (id=98581) [details]
Add core patch files for the battery status event.
------- Comment #4 From 2011-06-26 18:12:00 PST -------
Created an attachment (id=98651) [details]
Add core patch files for the battery status event
------- Comment #5 From 2011-06-27 10:08:39 PST -------
(From update of attachment 98651 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=98651&action=review

Looks OK, but ChangeLog entry needs work.

> Source/WebCore/ChangeLog:6
> +        [DOM] Core part patch for supporting battery status event
> +        https://bugs.webkit.org/show_bug.cgi?id=63376

"Add JavaScript wrappers for battery status events." would read better IMO. Also, things in brackets are usually used for port names. Since this is isn't port-specific, you can remove it.

> Source/WebCore/ChangeLog:14
> +        Add Core part modification for battery status event
> +        - JSEventCustom.cpp, V8EventCustom.cpp
> +          binding event between JavaSctipt and WebCore
> +        - dom/Document.cpp Event.cpp Event.h Eventnames.h
> +          Define and create event.
> +        - Page.h Page.cpp, DOMWindow.h DOMWindow.cpp
> +          Add/Remove event listener and init event handler to pageClient

These should go below, right after each file name.

> Source/WebCore/ChangeLog:18
> +        no new tests.
> +        This event implementation is not finished with this patch.
> +        After finishing inplemetation, It will be added.

Begin sentences with a capital letter. Better yet, rephrase this to say something like "Test cases will be added as soon as the whole battery status feature is implemented.".
------- Comment #6 From 2011-06-27 18:36:27 PST -------
Created an attachment (id=98840) [details]
Modify description in the ChangeLog
------- Comment #7 From 2011-06-27 18:37:29 PST -------
Thank you for your comment.
I modified my ChangeLog.

(In reply to comment #5)
> (From update of attachment 98651 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=98651&action=review
> 
> Looks OK, but ChangeLog entry needs work.
> 
> > Source/WebCore/ChangeLog:6
> > +        [DOM] Core part patch for supporting battery status event
> > +        https://bugs.webkit.org/show_bug.cgi?id=63376
> 
> "Add JavaScript wrappers for battery status events." would read better IMO. Also, things in brackets are usually used for port names. Since this is isn't port-specific, you can remove it.
> 
> > Source/WebCore/ChangeLog:14
> > +        Add Core part modification for battery status event
> > +        - JSEventCustom.cpp, V8EventCustom.cpp
> > +          binding event between JavaSctipt and WebCore
> > +        - dom/Document.cpp Event.cpp Event.h Eventnames.h
> > +          Define and create event.
> > +        - Page.h Page.cpp, DOMWindow.h DOMWindow.cpp
> > +          Add/Remove event listener and init event handler to pageClient
> 
> These should go below, right after each file name.
> 
> > Source/WebCore/ChangeLog:18
> > +        no new tests.
> > +        This event implementation is not finished with this patch.
> > +        After finishing inplemetation, It will be added.
> 
> Begin sentences with a capital letter. Better yet, rephrase this to say something like "Test cases will be added as soon as the whole battery status feature is implemented.".
------- Comment #8 From 2011-06-29 06:38:14 PST -------
(From update of attachment 98840 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=98840&action=review

> Source/WebCore/ChangeLog:44
> +        * bindings/js/JSEventCustom.cpp:
> +        (WebCore::toJS):
> +        * bindings/v8/custom/V8EventCustom.cpp:
> +        (WebCore::toV8):
> +        * dom/Document.cpp:
> +        (WebCore::Document::createEvent):
> +        * dom/Event.cpp:
> +        (WebCore::Event::isBatteryStatusEvent):
> +        * dom/Event.h:
> +        * dom/EventNames.h:
> +        * page/DOMWindow.cpp:
> +        (WebCore::DOMWindow::addEventListener):
> +        (WebCore::DOMWindow::removeEventListener):
> +        (WebCore::DOMWindow::removeAllEventListeners):
> +        * page/DOMWindow.h:
> +        * page/Page.cpp:
> +        (WebCore::Page::Page):
> +        (WebCore::Page::PageClients::PageClients):
> +        * page/Page.h:
> +        (WebCore::Page::batteryStatusController):

These are added so that you can fill them; no need for a separate section above, like so:

* page/Page.cpp: Add controller and client for...
...
* page/Page.cpp: Ditto.
------- Comment #9 From 2011-07-01 02:15:13 PST -------
Created an attachment (id=99446) [details]
Modify ChangeLog's comments
------- Comment #10 From 2011-07-01 03:49:22 PST -------
Created an attachment (id=99454) [details]
Modify comment of ChangLog
------- Comment #11 From 2011-07-06 23:43:12 PST -------
Created an attachment (id=99944) [details]
Add events for the revised spec.

http://dev.w3.org/2009/dap/system-info/battery-status.html
------- Comment #12 From 2012-02-16 14:05:12 PST -------
It seems there isn't much interest in this feature.
------- Comment #13 From 2012-02-17 00:04:57 PST -------
(In reply to comment #12)
> It seems there isn't much interest in this feature.

Battery Status API is almost close to CfC.
So. I will add a new patch for this soon.
------- Comment #14 From 2012-03-20 00:45:14 PST -------
(From update of attachment 99944 [details])
Cleared review? from attachment 99944 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).
------- Comment #15 From 2012-04-05 00:15:22 PST -------
Battery status API has already landed by bug #62698.