18444 - [FEATURE]Titledbuttons need to be focusable

Status: VERIFIED FIXED

(Core :: XUL, defect, P3)

Description

[xiaotong]

In dialog (e.g. Open Web location), using Tab key does not loop through
all the controls, it does not go to OK and Cancel buttons. There is no way
to reach OK and Cancel by using keyboard.

Comment 1

[shuang (gone)]

Looks like peter may have better sense about this...

Comment 2

[Peter Trudelle]

reassigning to sdagley as p3 for m13

Comment 3

[Steve Dagley]

Forgot to cc: hangas on this one in case it's related to the dialog overlay that
provides for the specialized behavior in ok/cancel buttons

Comment 4

[Peter Trudelle]

featuring

Comment 5

[Steve Dagley]

hyatt has been thwacking on tabbing code and said to re-assign tabbing bugs to
him

Comment 6

[David Hyatt]

Fixed, although titledbuttons with user-focus: ignore arent' smart enough
to ignore the tabbing.  This is true of HTML elements as well.

Comment 7

[Claudius Gayle]

hyatt, is this supposed to be in the 2000011308 build?

Comment 8

[Claudius Gayle]

well no matter, cuz with the 2000011408 comm opt builds this ain't fixed.
If i select File| Open web location then click in the text field or the checkbox
and hten try to tab over nothing happens except...

OH MY GOD!

is printed to the console for each tab press.

Comment 9

[David Hyatt]

You're right.  Thanks for pointing out the error message.  Led me right to the 
problem.  joki, I'd like you to review this little patch to 
GetNextTabbableContent.  I set hidden to PR_TRUE if I can't find a primary frame 
for the content that might be tabbed into.  This keeps us from tabbing into 
focusable content that has no associated frame (e.g., because of display: none).

With your approval, I'll check this puppy in when M14 opens.

The following code is inserted right after the if/else statements for all the 
form elements+trees+titledbuttons in GetNextTabbableContent.

// Get the primary frame for the widget.  We don't tab into anything
      // that doesn't have a frame.
      nsCOMPtr<nsIPresShell> shell;
      if (mPresContext) {
        nsresult rv = mPresContext->GetShell(getter_AddRefs(shell));
        if (NS_SUCCEEDED(rv) && shell){
          nsIFrame* potentialFrame;
          shell->GetPrimaryFrameFor(child, &potentialFrame);
          if (!potentialFrame)
            hidden = PR_TRUE;
        }
      }

Comment 10

[David Hyatt]

Fixed.

Comment 11

[Claudius Gayle]

was this fix part of your changes from last night (1/24) that are/were being backed out?

Comment 12

[Claudius Gayle]

this is VERIFIED on all platforms with the 2000020108 builds but I just wrote bug 26342 to address the fact that there is no visual 
indication of which widget has focus.