993361 - Caret browsing doesn't work well in XUL documents (such as about:preferences) and should be disabled there

Status: NEW

(Firefox :: Disability Access, defect, P4)

Description

[Camelia Badau [:cbadau], Desktop QA]

Reproducible on the latest Beta (BuildID:20140407135746)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20130205 Firefox/29.0
Reproducible on the latest Aurora (BuildID:20140407004002)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20130205 Firefox/30.0
Reproducible on the latest Nightly (BuildID: 20140407030203) 
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0

Steps to reproduce: 
1. Open about:preferences. 
2. Go to Advanced tab, on General and select the first option from Accesibility (“Always use the cursor keys to navigate within pages”)

Actual results: The following intermitent issue can be seen: a prompter ("|") appears in random places (on Advanced tab) in the left side of buttons, radio buttons and checkboxes.

Expected results: No issue occurs. 

Notes: 1.This is reproducible also on Ubuntu x86. 
       2.This issue is not a regression.

Comment 1

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

Alexander, do you know why the caret may be positioned so far from the checkbox's label? Can you give some guidance on how to fix this?

Comment 2

[alexander :surkov (:asurkov)]

I'm not sure who's the best person to help with wrong caret positioning. Mats, do you have ideas?

Comment 3

[Mats Palmgren (inactive)]

I'm confused.  The Summary says "in-content preferences" but this report
is filed on FF29,30,31 which doesn't have in-content preferences as far
as I can tell (on Linux).  Should this bug be about FF32, and FF29-31
is NOT affected?

Comment 4

[Mats Palmgren (inactive)]

(In reply to Camelia Badau, QA [:cbadau] from comment #0)
> Expected results: No issue occurs. 

In the future, please write out a description of what you expect
rather than just "no issue".  In this case I have to guess you
mean "no caret appears anywhere in about:preferences except
inside focused text controls".  Can you confirm?

Alexander's comment 2 seems to contradict that, since he says
the caret is just in the wrong position.  So it's unclear to
me what this bug is about.

Comment 5

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

(In reply to Mats Palmgren (:mats) from comment #3)
> I'm confused.  The Summary says "in-content preferences" but this report
> is filed on FF29,30,31 which doesn't have in-content preferences as far
> as I can tell (on Linux).  Should this bug be about FF32, and FF29-31
> is NOT affected?

in-content preferences have been in Firefox for a long time now. they were only recently made default (fx32). previously, one had to go to about:config and change the browser.preferences.inContent preference to 'true'.

The expected results are that the caret will appear in front of the first character of the checkbox labels.

The actual results are that the caret appears in front of the checkbox box, shifted down about 5 pixels as well.

Comment 6

[Mats Palmgren (inactive)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #5)
> The expected results are that the caret will appear in front of the first
> character of the checkbox labels.

Seems reasonable for XUL checkboxes that has a 'label' attribute.
I guess it means changes to the node traversal code for caret browsing
and I don't know where that code lives, sorry.

Maybe Ehsan or Masayuki knows?

Comment 7

[(no longer active)]

Sorry I'm having a hard time understanding what question you're asking.  If the question is, do you expect this to be buggy, then yes absolutely. :-)

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, I don't know where is the exact place. Anyway, caret navigation must not work well in XUL content. I think that caret shouldn't be appear on XUL document at least for now.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Looks like that you can fix around here if we would disable caret browsing on XUL document.
http://mxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp#2049

Comment 10

[Mats Palmgren (inactive)]

(In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment #7)
> Sorry I'm having a hard time understanding what question you're asking.

My question was, where is the source code that implements caret browsing
located in the tree?  (I'm guessing it might be possible to tweak it
to place the caret at the start of the label text for XUL checkboxes.)

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #9)
> Looks like that you can fix around here if we would disable caret browsing
> on XUL document.

Yeah, we could easily disable caret browsing for XUL, if that's an
acceptable solution.  Making caret browsing work well for XUL is
certainly going to require a lot of work and I don't think spending
resources on that serves our long term goals.

Comment 11

[Neil Deakin]

(In reply to Mats Palmgren (:mats) from comment #10)
> (In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment
> #7)
> > Sorry I'm having a hard time understanding what question you're asking.
> 
> My question was, where is the source code that implements caret browsing
> located in the tree?  (I'm guessing it might be possible to tweak it
> to place the caret at the start of the label text for XUL checkboxes.)
> 

When you focus an element, the caret is moved to the focused node. This is done by nsFocusManager::MoveCaretToFocus for non-mouse focuses and somewhere in nsSelection for mouse clicks. For checkboxes, the focused part is the square box so the caret will be positioned there.

Note that this happens regardless of whether caret browsing is on or off. The caret browsing preference only affects whether the caret is visible. So disabling caret browsing doesn't actually fix the real issue.

Comment 12

[(no longer active)]

Note that the caret movement part happens in nsFrameSelection::MoveCaret for all content (and that is the same code for both caret browsing and when we forcefully show the caret in editable areas).  That function is where most of the caret browsing bugs live.  :-)

Comment 13

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

Not considered a blocker for shipping the in-content prefs due to the low severity and low frequency of occurrence.

Comment 14

[Fischer [:Fischer]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #5)
> (In reply to Mats Palmgren (:mats) from comment #3)
> > I'm confused.  The Summary says "in-content preferences" but this report
> > is filed on FF29,30,31 which doesn't have in-content preferences as far
> > as I can tell (on Linux).  Should this bug be about FF32, and FF29-31
> > is NOT affected?
> 
> in-content preferences have been in Firefox for a long time now. they were
> only recently made default (fx32). previously, one had to go to about:config
> and change the browser.preferences.inContent preference to 'true'.
> 
> The expected results are that the caret will appear in front of the first
> character of the checkbox labels.
> 
> The actual results are that the caret appears in front of the checkbox box,
> shifted down about 5 pixels as well.

@Jared,
We are(from Taipei office), recently, reviewing and working on the pref-related bugs.
About this bug, after reading all the comments and descriptions, I am kind of confused what the issue is.
I would like to check with you my understanding right or not.

When check one checkbox on its label in the Advanced page through mouse clicking.
I can see the caret appears at the position of mouse click.
For example, clicking on "u" of "use", we would see the prompter("|") before "u", like "Always |use the cursor keys to navigate within pages".
However, this is not expected result.
The expected result should be that the caret appears in front of the first character, like "|Always use the cursor keys to navigate within pages" even it is "u" of "use" being clicked.

Is my understanding right ?

thanks.

Comment 15

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

Hi Fischer,

Thank you for your message. What you have described does not cover the main focus of this bug.

Here are some more detailed steps to reproduce this bug.

1. Open the preferences
2. Go to Advanced pane and enable the "Always use the cursor keys to navigate within pages" option
3. Press Tab to move the focus to the next checkbox.

Expected Result:
The cursor should appear at the beginning of the label, like so:
[] |Search for text when I start typing

Actual Result:
The cursor appears before the checkbox, like so:
|[] Search for text when I start typing

Comment 16

[Neil Deakin]

> Expected Result:
> The cursor should appear at the beginning of the label, like so:
> [] |Search for text when I start typing
> 
> Actual Result:
> The cursor appears before the checkbox, like so:
> |[] Search for text when I start typing

The 'actual result' is the same behaviour as checkboxes on webpages as well. Are you asking that be changed generally?

Comment 17

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

I didn't realize that until now.

That would mean the only remaining issue here is that the cursor is about 5 pixels lower than the webpage (non-XUL) version.

Comment 18

[:Gijs (he/him)]

(In reply to (unavailable 07/20-07/21) Jared Wein [:jaws] (please needinfo? me) from comment #17)
> I didn't realize that until now.
> 
> That would mean the only remaining issue here is that the cursor is about 5
> pixels lower than the webpage (non-XUL) version.

FWIW, I would agree with comment 8: we should just disable caret browsing for in-content XUL documents. Guidance for that is in comment #9. I don't think there's any in-content XUL where caret browsing is actually useful, and trying to improve it there isn't really a useful way to spend our time. If there's anything we want to do about this (because it looks odd in some places), we should just disable it for XUL outright. Jared, does that seem OK to you?

Comment 19

[Neil Deakin]

The preference pane currently allows text selection. I'm not sure if that's intentional or not, as UI doesn't normally allow this. The primary motivation for having caret browsing is to allow selections to be made by keyboard only users. So you probably want to enable or disable both together.

As comment 9 suggests, nsFocusManager::UpdateCaret would be the right place to disable the caret; note though that the showcaret attribute does not work in e10s so another mechanism handled in the content process would need to be added there, possibly a similar attribute on the root element.

Comment 20

[:Gijs (he/him)]

(In reply to Neil Deakin from comment #19)
> The preference pane currently allows text selection. I'm not sure if that's
> intentional or not, as UI doesn't normally allow this.

I would have no problems with disabling text selection on the UI elements there. I don't really know why it's even supported there when it isn't when XUL is in a window.

Comment 21

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

I agree that we should remove selections at the same time.