Closed Bug 722953 Opened 10 years ago Closed 10 years ago

Cause hardware buttons to illuminate when button is pressed


(Core :: Hardware Abstraction Layer (HAL), defect)

Gonk (Firefox OS)
Not set





(Reporter: jstraus, Assigned: jstraus)




(2 files, 1 obsolete file)

No description provided.
Cause the hardware buttons to illuminate when the buttons are pressed.  This will have a timer associated with it to cause the buttons to go off after a period.
Should these not be illuminated all the time the screen is on? I've seen users not notice there are back & menu buttons at all because they're not visible.
Would be great to get UX feedback on the question in comment 2.  This may not be a problem we need to solve in the near future.
That is an option.  Android supports a couple of different times, always on, always off, and based on the light sensor.
Assignee: nobody → jstraus
Patches the key dispatcher to enable the button lights for the menu and back buttons.  Turning on the lights triggers a timer to turn them off after a set period.  To make this completely useful, we'll need a javascript function to set the timeout.
Attachment #593718 - Flags: review?(jones.chris.g)
Depends on: 712378
Comment on attachment 593718 [details] [diff] [review]
Enables the button lights, with a timer to turn them off

This may be a good reason to create an ffi to let JS poke the lights.  Let's see what mwu thinks.
Attachment #593718 - Flags: review?(jones.chris.g) → review?(mwu)
Watch out for trailing whitespace. I use

diff.trailingwhitespace = bold red_background

in ~/.hgrc

and then hg diff (or hg qdiff)
Just FYI regarding comment 2, UX are going for a software back button in every app as a convention so having the hardware button illuminated all the time as I suggested may not be so important. I'm not sure about the menu button though, that wasn't clear.
Comment on attachment 593718 [details] [diff] [review]
Enables the button lights, with a timer to turn them off

Sorry about the review delay!

I don't think this sort of policy belongs in the widget backend. Some sort of FFI for JS to control turning lights on/off would be better.
Attachment #593718 - Flags: review?(mwu) → review-
Need some direction here. Chris?
Jim, let's twiddle the keyboard backlight inside hal::SetScreenBrightness(), in sync with the screen backlight.

Moving this out into content JS is a big can o' worms that I don't want to open right now, without clear use cases.
Ok, I cam move the function that actually twiddles the button lights to the hal area (GonkHal should do it).  nsAppShell will have to call the function, since that is where we can tell a button has been pressed.  Do you want the JS interface to the parameter in this patch?

On doing it from JS, SetLight() is defined in the previous patch, which could be used to implement this, but I agree, JS should set the parameter, but letting the underlying engine actually implement it seems more reasonable.
My suggestion is to make the button backlight operate in sync with the screen backlight.  I don't know of any use cases for controlling them separately yet, and operating in sync is the simplest implementation.
I was just emulating the functionality that Android provides, but doing it in sync with the screen is easy.
Restructured to move the button light handling and timer into the Hal area.  Added interface to allow JS to set the timeout.  Note that I consider SetLight to be a lower level interface to the lights and this to be a higher level way to handle the button lights.
Attachment #593718 - Attachment is obsolete: true
Attachment #597253 - Flags: review?(jones.chris.g)
Comment on attachment 597253 [details] [diff] [review]
Enables the button lights, with a timer to turn them off

This is the approach that mwu already r-'d.  It doesn't implement the follow-up suggestion in comment 11 and comment 13.  This duplicates what our idle service already does.

There's a very simple implementation of this, which treats the button backlight as part of the screen backlight.  There's also a very generic and flexible implementation which would fully expose the lights to content JS, the Gaia homescreen e.g.  Android makes use of this kind of interface to do things like turn on/off the key backlight when the lock screen is dismissed, and off idle timeouts.  We don't have a strong reason yet to add all this flexibility and doing so would be a fair amount of work, so it's not really worth the time atm.  Here we're somewhere in between, but with some hacks in gonk widgetry that mwu isn't in favor of, and some gonk hal hacks that I'm not in favor of.

I'm going to post a patch for the dead-simple impl.  Let's re-open this discussion if we get feedback from UX about button lighting.
Attachment #597253 - Flags: review?(jones.chris.g) → review-
(In reply to Andreas Gal :gal from comment #7)
> Watch out for trailing whitespace. I use
> [color]
> diff.trailingwhitespace = bold red_background
> in ~/.hgrc
> and then hg diff (or hg qdiff)

Git seems to warn about trailing whitespace if you put

    ui = true

in your gitconfig.  Also, git rebase --whitespace=fix is great.
Josh, not needed for demo milestone, but we should look at this for M3.

Cjones is looking for back light behavior of the screen vs. the buttons itself and how they tie together.
Comment on attachment 597280 [details] [diff] [review]
Turn on the button backlight along with the screen backlight

Nice simple fix. This is what we need. Please land.
Attachment #597280 - Flags: review?(jstraus) → review+
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.