Last Comment Bug 762349 - Allow Sleep button to bubble from browser frames
: Allow Sleep button to bubble from browser frames
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla16
Assigned To: Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)
:
Mentors:
Depends on: 755648 757486
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-06 20:25 PDT by Tim Guan-tin Chien [:timdream] (please needinfo)
Modified: 2012-06-12 22:41 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch to commit (v1) (1023 bytes, patch)
2012-06-06 20:27 PDT, Tim Guan-tin Chien [:timdream] (please needinfo)
justin.lebar+bug: review-
Details | Diff | Splinter Review
Patch to commit (v2) (1.81 KB, patch)
2012-06-06 20:56 PDT, Tim Guan-tin Chien [:timdream] (please needinfo)
justin.lebar+bug: review-
Details | Diff | Splinter Review
Patch v3 (2.41 KB, patch)
2012-06-06 21:36 PDT, Tim Guan-tin Chien [:timdream] (please needinfo)
justin.lebar+bug: review+
Details | Diff | Splinter Review
Patch to commit (2.41 KB, patch)
2012-06-06 21:59 PDT, Tim Guan-tin Chien [:timdream] (please needinfo)
no flags Details | Diff | Splinter Review
Patch to commit (3.12 KB, patch)
2012-06-06 22:04 PDT, Tim Guan-tin Chien [:timdream] (please needinfo)
justin.lebar+bug: review-
Details | Diff | Splinter Review
Patch v0.2 (3.15 KB, patch)
2012-06-11 09:19 PDT, Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)
justin.lebar+bug: review+
Details | Diff | Splinter Review

Description Tim Guan-tin Chien [:timdream] (please needinfo) 2012-06-06 20:25:27 PDT
+++ This bug was initially created as a clone of Bug #757486 +++

Per discussion on bug 755648 comment 25, add Sleep button to the whitelisted events for browser frames to bubble, feature introduced in Bug #757486.
Comment 1 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-06-06 20:27:31 PDT
Created attachment 630823 [details] [diff] [review]
Patch to commit (v1)

Patch v1, for commit, if r+
Comment 2 Justin Lebar (not reading bugmail) 2012-06-06 20:29:06 PDT
Comment on attachment 630823 [details] [diff] [review]
Patch to commit (v1)

It's kind of silly, but can you modify the test so we don't regress this?  (dom/tests/mochitest/test_browserFrame_keyEvents.html)
Comment 3 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-06-06 20:56:14 PDT
Created attachment 630834 [details] [diff] [review]
Patch to commit (v2)

Also add the sleep button to mochitest
Comment 4 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-06-06 20:56:44 PDT
(In reply to Justin Lebar [:jlebar] from comment #2)
> Comment on attachment 630823 [details] [diff] [review]
> Patch to commit (v1)
> 
> It's kind of silly, but can you modify the test so we don't regress this? 
> (dom/tests/mochitest/test_browserFrame_keyEvents.html)

Just did, thanks.
Comment 5 Justin Lebar (not reading bugmail) 2012-06-06 21:16:45 PDT
Comment on attachment 630834 [details] [diff] [review]
Patch to commit (v2)

That doesn't actually do anything; read the rest of the test.  :)  (Particularly, the bottom, under runTest().)
Comment 6 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-06-06 21:36:10 PDT
Created attachment 630842 [details] [diff] [review]
Patch v3

Now this should make sense ...
Comment 7 Justin Lebar (not reading bugmail) 2012-06-06 21:47:18 PDT
Comment on attachment 630842 [details] [diff] [review]
Patch v3

I think you also need to modify nbEvents, but yes.  :)
Comment 8 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-06-06 21:56:29 PDT
(In reply to Justin Lebar [:jlebar] from comment #7)
> I think you also need to modify nbEvents, but yes.  :)

What do you mean? Do I need to make more modification?
Comment 9 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-06-06 21:59:07 PDT
Created attachment 630846 [details] [diff] [review]
Patch to commit

If there is no further modification required, this is the patch to commit.
Comment 10 Justin Lebar (not reading bugmail) 2012-06-06 22:02:30 PDT
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TW) from comment #8)
> (In reply to Justin Lebar [:jlebar] from comment #7)
> > I think you also need to modify nbEvents, but yes.  :)
> 
> What do you mean? Do I need to make more modification?

Yes, I think so.  See |var nbEvents = 15;|.
Comment 11 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-06-06 22:04:47 PDT
Created attachment 630847 [details] [diff] [review]
Patch to commit

nbEvents=16
Comment 12 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-06-06 22:06:38 PDT
@jliebar suggests this could go through try-server before landing. @vingtetun, @mounir, thought?
Comment 13 Mounir Lamouri (:mounir) 2012-06-07 01:14:43 PDT
*Everything* has to go through try server before being pushed to m-i or m-c.
Comment 14 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2012-06-07 03:47:41 PDT
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TW) from comment #12)
> @jliebar suggests this could go through try-server before landing.
> @vingtetun, @mounir, thought?

Tim do you have a Level 1 access ? (http://www.mozilla.org/hacking/commit-access-policy/)
Comment 15 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-06-07 04:00:19 PDT
(In reply to Vivien Nicolas (:vingtetun) from comment #14)
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TW) from comment #12)
> > @jliebar suggests this could go through try-server before landing.
> > @vingtetun, @mounir, thought?
> 
> Tim do you have a Level 1 access ?
> (http://www.mozilla.org/hacking/commit-access-policy/)

I do (bug 726492), but I've never use it and I got Error when I tried to push to try:

$ hg push -f ssh://hg.mozilla.org/try/ -e 'ssh -l tchien@mozilla.com -i ~/mozilla_id_rsa'
pushing to ssh://hg.mozilla.org/try/
remote: Permission denied (publickey,keyboard-interactive).
abort: no suitable response from remote hg!
Comment 16 Justin Lebar (not reading bugmail) 2012-06-07 06:17:54 PDT
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TW) from comment #15)
> (In reply to Vivien Nicolas (:vingtetun) from comment #14)
> > (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TW) from comment #12)
> > > @jliebar suggests this could go through try-server before landing.
> > > @vingtetun, @mounir, thought?
> > 
> > Tim do you have a Level 1 access ?
> > (http://www.mozilla.org/hacking/commit-access-policy/)
> 
> I do (bug 726492), but I've never use it and I got Error when I tried to
> push to try

Please re-open your L1 access bug, or ping someone in #it.  In the meantime, I'll push this to try for you.
Comment 17 Justin Lebar (not reading bugmail) 2012-06-07 06:21:51 PDT
https://tbpl.mozilla.org/?tree=Try&rev=56f4f21a083a
Comment 18 Justin Lebar (not reading bugmail) 2012-06-07 11:21:29 PDT
Comment on attachment 630847 [details] [diff] [review]
Patch to commit

Test failures on try (all platforms): https://tbpl.mozilla.org/php/getParsedLog.php?id=12452334&tree=Try
Comment 19 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2012-06-11 02:24:25 PDT
(In reply to Justin Lebar [:jlebar] from comment #18)
> Comment on attachment 630847 [details] [diff] [review]
> Patch to commit
> 
> Test failures on try (all platforms):
> https://tbpl.mozilla.org/php/getParsedLog.php?id=12452334&tree=Try

I have pushed a new version on https://tbpl.mozilla.org/?tree=Try&rev=705f6780206a

I bet nbEvents should be whitelistedEvents.length * 3 instead of an hardcoded number.
Comment 20 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2012-06-11 09:18:37 PDT
(In reply to Vivien Nicolas (:vingtetun) from comment #19)
> (In reply to Justin Lebar [:jlebar] from comment #18)
> > Comment on attachment 630847 [details] [diff] [review]
> > Patch to commit
> > 
> > Test failures on try (all platforms):
> > https://tbpl.mozilla.org/php/getParsedLog.php?id=12452334&tree=Try
> 
> I have pushed a new version on
> https://tbpl.mozilla.org/?tree=Try&rev=705f6780206a
> 
> I bet nbEvents should be whitelistedEvents.length * 3 instead of an
> hardcoded number.

After a few typos this one sounds green: https://tbpl.mozilla.org/?tree=Try&rev=bef95aa45b44 (still need to run a little thought...)
Comment 21 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2012-06-11 09:19:47 PDT
Created attachment 631911 [details] [diff] [review]
Patch v0.2
Comment 22 [:fabrice] Fabrice Desré 2012-06-11 13:00:40 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/a6321d945bf2
Comment 23 Graeme McCutcheon [:graememcc] 2012-06-12 03:05:30 PDT
https://hg.mozilla.org/mozilla-central/rev/a6321d945bf2
Comment 24 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-06-12 22:41:43 PDT
@vingtetun Thanks.

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