1373570 - "Update to ..." button in the about dialog cannot get focus after Bug 1365133 is fixed

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[Evan Tseng [:evanxd]]

Do not focus the "Check for updates" button when going into Preferences page, but we want it at about dialog. See also https://bugzilla.mozilla.org/show_bug.cgi?id=1365133#c20.

Comment 1

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Is this bug still need fixing once bug 1374852 landed? if not let's WFM here.

Comment 2

[Evan Tseng [:evanxd]]

Yes, we still need to fix this bug. Bug 1374852 don't fix this.

Comment 3

[Evan Tseng [:evanxd]]

Attachment: Bug 1373570 - Focus the update button in about dialog once there is one visible.

Review commit: https://reviewboard.mozilla.org/r/153078/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/153078/

Comment 4

[Evan Tseng [:evanxd]]

Hi Jared,

The focus function will be lost after Bug 1365133's (Reorg v2) patch is landed. So let's fix it.

Could you help review the patch?

Thank you.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8881997 [details]
Bug 1373570 - Focus the update button in about dialog once there is one visible.

https://reviewboard.mozilla.org/r/153078/#review158528

Comment 6

[Evan Tseng [:evanxd]]

Thank you for reviewing, Jared.

We will land the patch after Bug 1365133 is fixed.

Comment 7

[Evan Tseng [:evanxd]]

Comment on attachment 8881997 [details]
Bug 1373570 - Focus the update button in about dialog once there is one visible.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153078/diff/1-2/

Comment 8

[Evan Tseng [:evanxd]]

Rebased the patch. Once the try[1] is good, let's land the patch.

[1]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=51021f5d0ca8

Comment 9

[Evan Tseng [:evanxd]]

Looks like the try is good. Let's land the patch.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/af4655e3719a
Focus the update button in about dialog once there is one visible. r=jaws

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/af4655e3719a

Comment 12

[Deac Alin]

(In reply to Evan Tseng [:evanxd] 
> Do not focus the "Check for updates" button when going into Preferences
> page, but we want it at about dialog. 

I am trying to verify this bug, but i could not find the "Check for updates" button in the about dialog section (only in Preferences section). The button still exists in the latest builds? 
Thank you

Comment 13

[Evan Tseng [:evanxd]]

Attachment: bug-1373570.png

Hi Deac,

You can make the "Check for updates" button visible by adding a new pref "app.update.elevate.never" as true. See the attachment.

And you should verify this in the about dialog not in the preferences page. See the attachment also.

Thank you for verifying.

Comment 14

[Evan Tseng [:evanxd]]

The "Check for updates" button should get focus after opening the about dialog. That behavior is what we want and solved here.

Comment 15

[Deac Alin]

Attachment: Screen Shot 2017-08-16 at 11.04.21.png

Thank you Evan for the additional info.
Managed to verify this bug on the latest Nightly 57.0a1 (2017-08-15) (64-bit). Here are my findings:

-On Windows 10 x64 - The "Check for updates" button is properly focused after opening the about dialog (visible blue border around the button and by pressing ENTER key - the button is functional). Also the mouse hover is properly displayed.
-On Mac OSX 10.12.5 - The "Check for updates" button is not properly focused (grey border around the button - check screenshot) and by pressing ENTER key the button is not triggered (as intended for focused element). Mouse hover is not active as well.
-On Ubuntu 16.04 x64 - The "Check for updates" button has no visible border. Mouse hover not activating the button. Also by pressing ENTER key - the button is pressed (activated).

Comment 16

[Deac Alin]

Should we reopen this bug? considering the behaviour of The "Check for updates" button on Mac and Ubuntu(described above)

Comment 17

[Evan Tseng [:evanxd]]

Hi Deac,

(In reply to Deac Alin from comment #16)
> Should we reopen this bug? considering the behaviour of The "Check for
> updates" button on Mac and Ubuntu(described above)
No, I think we should not do it since we have Bug 1390561 to address the functionality issue. This bug only fixes the focus issue. In this bug, I think the only thing we should verify is checking the "Check for updates" button have focus state when open the about dialog.

> -On Mac OSX 10.12.5 - The "Check for updates" button is not properly focused (grey border around the button - check screenshot) and by pressing ENTER key the button is not triggered (as intended for focused element). Mouse hover is not active as well.
I cannot reproduce this. In my mac, I can see the button have focus after opening the about dialog.

Comment 18

[Deac Alin]

(In reply to Evan Tseng [:evanxd] from comment #17)
> I cannot reproduce this. In my mac, I can see the button have focus after
> opening the about dialog.

Are you using Mac OSX 10.10 by any chance? I observed that the mentioned issue is reproducible only on 10.12, and working fine on 10.10(the button has focus).

Comment 19

[Evan Tseng [:evanxd]]

My version of macOS is 10.12.5 and I can see the button have focus after opening the about dialog.

Comment 20

[Hani Yacoub, Desktop QA]

Attachment: ubuntu 16.04.png

The button is focused on Windows 10, on Mac OS X 10.12 and Mac OS X 10.10 but is not focused on Ubuntu 16.04 as Alin mentioned in comment 15. (Please see the screenshot attached)

Comment 21

[Deac Alin]

Indeed, it seems that only on my machine( Mac OSX 10.12.5) the button is not focused. I checked on 2 different 10.12 systems with Hani, and it's all good. The Ubuntu issue still stands tho.

Comment 22

[Evan Tseng [:evanxd]]

Hi Deac,

Are you saying the button doesn't have the focus on Ubuntu?

Comment 23

[Evan Tseng [:evanxd]]

I'll test it tomorrow. Thank you.

Comment 24

[Evan Tseng [:evanxd]]

Hi Deac,

I've tried this on my Ubuntu machine and I cannot reproduce this issue.

Could you find a new Ubuntu machine and try it again?

Thank you.

Comment 25

[Deac Alin]

Hi Evan,
I tested on a different machine with same result as described above by me and Hani(via screenshot).
- On Ubuntu 16.04 x64, using the latest Nightly - The "Check for updates" button has no visible border. Mouse hover not activating the button. Also by pressing ENTER key - the button is pressed (activated) - Is this enough and intended? As by pressing Enter Key the button is focused and functional.

Comment 26

[Hani Yacoub, Desktop QA]

I tried to verify this bug and bug 1390561 again on the latest nightly 57.0a1 and here are the conclusions:

1. On windows 10 x 64: The "Check for updates" button is properly focused after opening the about dialog (visible blue border around the button and by pressing ENTER key - the button is functional). Also the mouse hover is properly displayed.

2. On Mac OS 10.10 & Mac OS 10.12: The "Check for updates" button is focused, the mouse hover is not displayed and by pressing ENTER key this button is not functional.

3. On Ubuntu 16.04: The "Check for updates" button is NOT focused, the mouse hover is not displayed but by pressing ENTER key the button is functional.

What we should do in this case? Could we mark this bug as verified at least on Windows and log new bug for Mac and for Ubuntu?

Thanks.

Comment 27

[Evan Tseng [:evanxd]]

(In reply to Hani Yacoub from comment #26)
> I tried to verify this bug and bug 1390561 again on the latest nightly
> 57.0a1 and here are the conclusions:
> 
> 1. On windows 10 x 64: The "Check for updates" button is properly focused
> after opening the about dialog (visible blue border around the button and by
> pressing ENTER key - the button is functional). Also the mouse hover is
> properly displayed.
> 
> 2. On Mac OS 10.10 & Mac OS 10.12: The "Check for updates" button is
> focused, the mouse hover is not displayed and by pressing ENTER key this
> button is not functional.
As my understand, we only could press space key to trigger the function of the button.

> 3. On Ubuntu 16.04: The "Check for updates" button is NOT focused, the mouse
> hover is not displayed but by pressing ENTER key the button is functional.
I'm not sure what do you mean of this, "the mousehover is not displayed"? Could you tell me more details?

> What we should do in this case? Could we mark this bug as verified at least
> on Windows and log new bug for Mac and for Ubuntu?
> 
> Thanks.

Could you try to verify bug 1390561 with the STR mentioned at https://bugzilla.mozilla.org/show_bug.cgi?id=1390561#c26?
I update this bug's title and that is what we want to fix here.

Thank you.

Comment 28

[Hani Yacoub, Desktop QA]

(In reply to Evan Tseng [:evanxd] (Away 9/2 - 9/11) from comment #27)
> (In reply to Hani Yacoub from comment #26)
> > I tried to verify this bug and bug 1390561 again on the latest nightly
> > 57.0a1 and here are the conclusions:
> > 
> > 1. On windows 10 x 64: The "Check for updates" button is properly focused
> > after opening the about dialog (visible blue border around the button and by
> > pressing ENTER key - the button is functional). Also the mouse hover is
> > properly displayed.
> > 
> > 2. On Mac OS 10.10 & Mac OS 10.12: The "Check for updates" button is
> > focused, the mouse hover is not displayed and by pressing ENTER key this
> > button is not functional.
> As my understand, we only could press space key to trigger the function of
> the button.

On Mac OS's only be pressing space key we could trigger the function of the button, but on other OS's we could trigger the function of the button by pressing Enter key also.
It's still not that clear if it's the intended behavior?

> 
> > 3. On Ubuntu 16.04: The "Check for updates" button is NOT focused, the mouse
> > hover is not displayed but by pressing ENTER key the button is functional.
> I'm not sure what do you mean of this, "the mousehover is not displayed"?
> Could you tell me more details?

I was referring to the highlight displayed when I hover on the focused button. This highlight is visible only on Windows and I don't know it is should be on other OS's?

> > What we should do in this case? Could we mark this bug as verified at least
> > on Windows and log new bug for Mac and for Ubuntu?
> > 
> > Thanks.
> 
> Could you try to verify bug 1390561 with the STR mentioned at
> https://bugzilla.mozilla.org/show_bug.cgi?id=1390561#c26?
> I update this bug's title and that is what we want to fix here.
> 
> Thank you.

Thanks.

Comment 29

[Evan Tseng [:evanxd]]

> On Mac OS's only be pressing space key we could trigger the function of the
> button, but on other OS's we could trigger the function of the button by
> pressing Enter key also.
> It's still not that clear if it's the intended behavior?
In my understanding, it is default behavior.

> I was referring to the highlight displayed when I hover on the focused
> button. This highlight is visible only on Windows and I don't know it is
> should be on other OS's?
I think it is also default behavior. I test this with Firefox 55 (before this patch is landed) on macOS, there is no hover effect for the "Check for updates" and "Update to ..." buttons.

Comment 30

[Evan Tseng [:evanxd]]

I've tested this with Firefox 55 (before this patch is landed) on macOS, there is no hover effect for the "Check for updates" and "Update to ..." buttons.

Comment 31

[Hani Yacoub, Desktop QA]

Build ID: 20170913220121
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0

Verified as fixed on Firefox Nightly 57.0a1 on Windows 10 x 64, Mac OS X 10.12 and Ubuntu 16.04 x64.