Closed Bug 859275 Opened 11 years ago Closed 10 years ago

Don't invoke the skb when opening a new tab

Categories

(Firefox for Metro Graveyard :: App Bar, defect)

x86_64
Windows 8.1
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: jimm, Unassigned)

Details

Attachments

(1 obsolete file)

STR:

1) open a new tab

result: Firefox Start is displayed, focus is set to the nav bar, and all text is selected. Soft keyboard is displayed.

expected: focus should be in the nav bar for typing with a keyboard, but the soft keyboard shouldn't come up unless the user taps in the nav bar first.
Attached patch fix (obsolete) — Splinter Review
Assignee: nobody → jmathies
Attachment #734566 - Flags: review?(mbrubeck)
This and a fix for bug 851592 would make FS a far more user friendly screen on touch only tablets.
Comment on attachment 734566 [details] [diff] [review]
fix

Review of attachment 734566 [details] [diff] [review]:
-----------------------------------------------------------------

With this patch, we don't seem to focus the URL field in a new tab, so this regresses the behavior for hardware keyboards.  I'm not sure of the right way to fix this.
Attachment #734566 - Flags: review?(mbrubeck) → review-
(In reply to Matt Brubeck (:mbrubeck) from comment #3)
> Comment on attachment 734566 [details] [diff] [review]
> fix
> 
> Review of attachment 734566 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> With this patch, we don't seem to focus the URL field in a new tab, so this
> regresses the behavior for hardware keyboards.  I'm not sure of the right
> way to fix this.

Hmm. I'm not able to reproduce that. Clicking or taping the + button creates a tab, opens the start page, and focuses the url bar. I'm able to type using a keyboard immediately. I wonder if there's some weird timing going on or something between our machines.
Comment on attachment 734566 [details] [diff] [review]
fix

Oh wait, I applied the patch wrong (by hand).  You changed cmd_openLocation, which is only used by the Control-L and Alt-D keyboard shortcuts:

>       case "cmd_openLocation":
>         ContextUI.displayNavbar();
>-        this._editURI();
>         break;

To change the "New Tab" behavior, you want cmd_newTab.

I think the behavior of the current code in cmd_newTab is intermittent.  I'm guessing your patch didn't actually change the behavior, but you happened to be "lucky" when testing it.

I think calling focus() should never automatically show the software keyboard (per Microsoft's UX guidelines), so we should probably find out why it sometimes does, and fix that.
Odd, fixing cmd_openLocation fixed the problem reliably for me.

The reason why the keyboard comes is a side effect of how we interact with windows UIA. Basically what happens is:

1) user taps plus
2) mouse events fire
3) front end code sets focus to text edit
4) ms queries our backend through accessibility for the current focus element
5) accessibility says it's an editable element
6) windows brings up the keyboard

I would think that Windows might cross check the coordinates of the tap with the dims on the focus element, but apparently they don't or at least they are pretty loose about enforcing it.

I can test a bit more to see if they check at all to make sure it's not a bug in our UIA bridge code.
Attachment #734566 - Attachment is obsolete: true
Assignee: jmathies → nobody
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WORKSFORME
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: