1382222 - stylo: setting border on element prevents active element indicator being shown when using keyboard natigation

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P2)

Description

[Ryan Jones-Ward [:sciguyryan]]

Attachment: reduced testcase.html

Hi all!

I found this one while editing a post on Mozillazine. I am unable to test how this behaves in standalone Servo as it doesn't currently support tab switching for input elements (see https://github.com/servo/servo/issues/3982). I have however tested this with Chrome and with Servo CSS disabled for comparison. I have provided reference screenshots for both of these along with the reduced testcase for consideration.

The title is a bit convoluted but I couldn't think of a good way of explaining this.

Loading the testcase and clicking on the textarea then hitting tab to switch the active element to the button will switch to the button but with no visual indication that it is selected at all. The fact that the element is actually selected can be demonstrated by following the steps above and hitting the enter key - an alert will be shown that is set via the onclick event on the button.

I don't know how much of a problem this is but it seems as though it could cause a security problem if someone were not sure which element they were clicking on.

Comment 1

[Ryan Jones-Ward [:sciguyryan]]

Attachment: Reference Render With Chrome.PNG

Comment 2

[Ryan Jones-Ward [:sciguyryan]]

Attachment: Reference Render With Servo CSS Disabled.PNG

Comment 3

[Ryan Jones-Ward [:sciguyryan]]

I'm not sure who the best to ni is in this instance. I'm not exactly clear who is mainly in charge of Stylo stuff at the moment!

Comment 4

[Cameron McCormack (:heycam)]

Thanks Ryan, yes I can reproduce the problem.

The focus ring is rendered using an additional style context on the button frame, and we currently don't restyle those (bug 1366721).  I'm pretty sure that's the problem.

Comment 6

[Simon Sapin (:SimonSapin)]

Per bug 1382687, the border is normally set by a `input[type="submit"]:-moz-focusring::-moz-focus-inner` CSS rule in the user-agent stylesheet. I’ve tested that both :-moz-focusring and ::-moz-focus-inner work when used separately (in a UA sheet) but not together.

Comment 7

[Ryan Jones-Ward [:sciguyryan]]

I can confirm that the fix as posted in bug 1366721 fixes this by grabbing the version that has the patch from autoland.

Comment 8

[Ryan Jones-Ward [:sciguyryan]]

I'm going to mark this one as resolved -> fixed now since the resolution of bug 1366721 has also resolved this.

I'm going to ni heycam to see if we will need a testcase from this bug to compliment the one added in the patch for bug 1366721.

I couldn't think of a good way to build the testcase here into one that would fit in the reftest framework - setting .focus() on an element doesn't appear to emulate the act of tabbing between form elements.

Comment 9

[Cameron McCormack (:heycam)]

(In reply to Ryan Jones [:sciguyryan] from comment #8)
> I'm going to ni heycam to see if we will need a testcase from this bug to
> compliment the one added in the patch for bug 1366721.

Thanks Ryan.  In the end, I think the existing 731726-1.html test that bug 1366721 made pass looks to be sufficient.