Closed Bug 885383 Opened 7 years ago Closed 7 years ago

Work - Selection in non-focused inputs does not display

Categories

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

x86_64
Windows 8.1
defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 26

People

(Reporter: jimm, Assigned: kjozwiak)

References

Details

(Whiteboard: [selection][shovel-ready] feature=work)

Attachments

(2 files, 2 obsolete files)

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.
Blocks: metrov1defect&change
No longer blocks: metrov1triage
Summary: Selection in non-focused inputs does not display → Defect - Selection in non-focused inputs does not display
Whiteboard: [shovel-ready] → [shovel-ready] feature=Defect c=tbd u=tbd p=0
We've decided the default behavior should be to clear selection when an input loses focus.
Priority: -- → P3
Blocks: 855434
No longer blocks: metrov1defect&change
Summary: Defect - Selection in non-focused inputs does not display → Work - Selection in non-focused inputs does not display
Whiteboard: [shovel-ready] feature=Defect c=tbd u=tbd p=0 → [shovel-ready] feature=work
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?
Flags: needinfo?(jmathies)
(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.
Flags: needinfo?(jmathies)
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.
14:42:25] <jimm> bbondy: we could probably listen for tab key events, then use document.activeElement to turn selection on.
Gotcha, thanks for the information Brian. I will assign this to myself and give it a stab!
Assignee: nobody → kamiljoz
Whiteboard: [shovel-ready] feature=work → [selection][shovel-ready] feature=work
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
Attachment #792602 - Flags: review?(jmathies)
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.
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!
Attachment #792602 - Attachment is obsolete: true
Attachment #792602 - Flags: review?(jmathies)
Attachment #796645 - Flags: review?(jmathies)
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.
Attachment #796645 - Flags: review?(jmathies) → review+
added the changes as per comment #10 and went through all the test cases ad before.
Attachment #796645 - Attachment is obsolete: true
Attachment #797012 - Flags: review+
Jim,

Could you help me with pushing to try and landing?
Flags: needinfo?(jmathies)
(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
Flags: needinfo?(jmathies)
Thanks for running the tests! I think the test failures in the try push are unrelated to the bug, could you confirm?
(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.
Jim, thanks for running it again! Looks good
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/4bbd61c5fa03
Keywords: checkin-needed
Whiteboard: [selection][shovel-ready] feature=work → [selection][shovel-ready] feature=work[fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/4bbd61c5fa03
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [selection][shovel-ready] feature=work[fixed-in-fx-team] → [selection][shovel-ready] feature=work
Target Milestone: --- → Firefox 26
Depends on: 947214
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.