885383 - Work - Selection in non-focused inputs does not display

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Theme, defect, P3)

Description

[Jim Mathies [:jimm]]

Attachment: two text areas testcase

STR:

1) select text in the lower text area using the mouse
2) click the upper text area

result: selection goes away in lower text area

3) tab

result: selection comes back

The background highlight for selection in non-focused edits is messed up.

note, desktop has the same behavior. however we have selection monocles that stay active which makes this look weird. We should highlight the selection or clear selection completely on blur.

Comment 1

[Jim Mathies [:jimm]]

We've decided the default behavior should be to clear selection when an input loses focus.

Comment 2

[Kamil Jozwiak [:kjozwiak]]

Jim, went through the original test case on two different machines and they both behave the same as the desktop (the second machine was a touch device). When you select all of the text from the bottom text box, select the top text box and then tab over, it will move down to the bottom text box and have everything selected.

Tried this with the x1 carbon (touch) and it seems to be working the same as the desktop. The monocles are not being left behind. Is this the correct behavior? Should I mark this as Works For Me?

Comment 3

[Jim Mathies [:jimm]]

(In reply to Kamil Jozwiak [:kjozwiak] from comment #2)
> Jim, went through the original test case on two different machines and they
> both behave the same as the desktop (the second machine was a touch device).
> When you select all of the text from the bottom text box, select the top
> text box and then tab over, it will move down to the bottom text box and
> have everything selected.
> 
> Tried this with the x1 carbon (touch) and it seems to be working the same as
> the desktop. The monocles are not being left behind. Is this the correct
> behavior? Should I mark this as Works For Me?

not yet, we still need to get selection clearing going when inputs lose focus.

Comment 4

[Brian R. Bondy [:bbondy]]

Just talked to jimm on irc, this bug is about the selection monocles not re-appearing when you tab back to the first box. And there's no way to re-add the monocles. 

One way may be to re-add the selection monocles when you tab back, but if that is too hard then just clear the selection as Jim said.

Comment 5

[Brian R. Bondy [:bbondy]]

14:42:25] <jimm> bbondy: we could probably listen for tab key events, then use document.activeElement to turn selection on.

Comment 6

[Kamil Jozwiak [:kjozwiak]]

Gotcha, thanks for the information Brian. I will assign this to myself and give it a stab!

Comment 7

[Kamil Jozwiak [:kjozwiak]]

Attachment: Monocles don't appear when tabbing into an edit field

Went through several tests to make sure everything worked and behaved correctly:

- Went back and forth between the text boxes attached above using a mosue and ensured that the behavior was correct
- Went back and forth between the text boxes attached above using touch and ensured that the behavior was correct
- Tabbed through the text boxes and ensured that the monocles where present without any issues

Comment 8

[Jim Mathies [:jimm]]

Comment on attachment 792602 [details] [diff] [review]
Monocles don't appear when tabbing into an edit field

Can you make use if the editable detection utilities for this instead?

http://mxr.mozilla.org/mozilla-central/source/browser/metro/base/content/Util.js#196

Also, please break this keydown code out into a method.

Comment 9

[Kamil Jozwiak [:kjozwiak]]

Attachment: Monocles don't appear when tabbing into an edit field #2

Added all the changes as per comment #8 and went through the same test cases as per comment #7 without any issues. Let me know if there's anything else that needs to be changed!

Comment 10

[Jim Mathies [:jimm]]

Comment on attachment 796645 [details] [diff] [review]
Monocles don't appear when tabbing into an edit field #2

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

::: browser/metro/base/content/input.js
@@ +172,5 @@
> +          aEvent.target.selectionStart != aEvent.target.selectionEnd) {
> +        SelectionHelperUI.closeEditSession(false);
> +      }
> +      setTimeout(function() {
> +        let element = Browser.selectedBrowser.contentDocument.activeElement;

above this please add little comment:

// not e10s friendly

help us track down the spots we'll need to touch up when we switch to remote content.

@@ +182,5 @@
> +            !SelectionHelperUI.isActive &&
> +            element.selectionStart != element.selectionEnd &&
> +            aEvent.target != element) {
> +              let rect = element.getBoundingClientRect();
> +              SelectionHelperUI.attachEditSession(Browser.selectedTab.browser,

can you use Browser.selectedBrowser here? That would match up with where you got the element from above.

Comment 11

[Kamil Jozwiak [:kjozwiak]]

Attachment: Monocles don't appear when tabbing into an edit field #3

added the changes as per comment #10 and went through all the test cases ad before.

Comment 12

[Kamil Jozwiak [:kjozwiak]]

Jim,

Could you help me with pushing to try and landing?

Comment 13

[Jim Mathies [:jimm]]

(In reply to Kamil Jozwiak [:kjozwiak] from comment #12)
> Jim,
> 
> Could you help me with pushing to try and landing?

sure - 

https://tbpl.mozilla.org/?tree=Try&rev=bbc3901ddc88

Comment 14

[Kamil Jozwiak [:kjozwiak]]

Thanks for running the tests! I think the test failures in the try push are unrelated to the bug, could you confirm?

Comment 15

[Jim Mathies [:jimm]]

(In reply to Kamil Jozwiak [:kjozwiak] from comment #14)
> Thanks for running the tests! I think the test failures in the try push are
> unrelated to the bug, could you confirm?

Odd, you usually don't get that many failures on a push, our tests are pretty stable. I retriggered on that push, pulled tip again and repushed - 

https://tbpl.mozilla.org/?tree=Try&showall=0&rev=1b3c8042e395

lets see how this one comes out.

Comment 16

[Kamil Jozwiak [:kjozwiak]]

Jim, thanks for running it again! Looks good

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/4bbd61c5fa03

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4bbd61c5fa03