Closed
Bug 858206
Opened 12 years ago
Closed 11 years ago
Drag selection monocles should not push other monocles out of the way
Categories
(Firefox for Metro Graveyard :: Input, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 30
People
(Reporter: jimm, Assigned: azasypkin)
References
Details
(Whiteboard: [selection] p=5 s=it-30c-29a-28b.3 r=ff30 [qa-])
Attachments
(3 files, 7 obsolete files)
(deleted),
patch
|
jimm
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
azasypkin
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jimm
:
review+
|
Details | Diff | Splinter Review |
To better match Win8 and android behavior.
str:
1) select some text
2) drag one monocle toward the other
result: the other monocle gets push out of the way
desired: start and end nodes should reverse.
Reporter | ||
Updated•11 years ago
|
OS: Windows 8 Metro → Windows 8.1
Updated•11 years ago
|
Whiteboard: [selection] → [selection] p=0
Updated•11 years ago
|
Priority: -- → P1
Whiteboard: [selection] p=0 → [selection] p=5
Updated•11 years ago
|
Assignee: nobody → azasypkin
Blocks: metrobacklog
Status: NEW → ASSIGNED
QA Contact: kamiljoz
Whiteboard: [selection] p=5 → [selection] p=5 s=it-30c-29a-28b.3 r=ff30
Reporter | ||
Comment 3•11 years ago
|
||
Getting this fixed should solve a number of problems. One annoyance I keep running into is that sometimes when you try to drag a single monocle when both monocles are close, you end you end up dragging both. This happens often when transitioning from caret to selection mode.
I'm hoping this will address the issue by allowing the monocle you actually grab to transition across the other. Monocle z-level probably plays a role here.
Assignee | ||
Comment 4•11 years ago
|
||
Thanks for the input! Yes, I'm trying to cover as much cases as possible. A bunch of different selection issues are related to this in one or another way.
Assignee | ||
Comment 5•11 years ago
|
||
Initial patch version. Still doesn't work right for inputs with out-of-bound selection.
Assignee | ||
Comment 6•11 years ago
|
||
Assignee | ||
Comment 7•11 years ago
|
||
Added some WIP patches, still working on out-of-bound selection for inputs and some other oddities.
Assignee | ||
Comment 8•11 years ago
|
||
Still poking around _handleSelectionPoint trying to improve and simplify it. Also have problems with textareas. Probably it can be addressed in the separate bug.
Patch is mainly oriented on inputs as I have several dependent bugs.
Attachment #8386052 -
Attachment is obsolete: true
Attachment #8387403 -
Flags: feedback?(jmathies)
Assignee | ||
Comment 9•11 years ago
|
||
Attachment #8386053 -
Attachment is obsolete: true
Attachment #8387404 -
Flags: feedback?(jmathies)
Assignee | ||
Comment 10•11 years ago
|
||
Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=8246b36a0d9a
Attachment #8387405 -
Flags: feedback?(jmathies)
Reporter | ||
Comment 11•11 years ago
|
||
This is acting kind of strange in text areas -
1) off an initial drag, I always seem to select from the caret position down to the the line below the caret position.
2) selection seems to jump around selecting larger blocks that I'm aiming for.
3) dragging across the other monocle doesn't reverse selection, it still seems to drag the other monocle with it resulting in a single character of selection dragging under my finger.
Assignee | ||
Comment 12•11 years ago
|
||
Attachment #8387403 -
Attachment is obsolete: true
Attachment #8387403 -
Flags: feedback?(jmathies)
Attachment #8388696 -
Flags: feedback?(jmathies)
Assignee | ||
Comment 13•11 years ago
|
||
Attachment #8387404 -
Attachment is obsolete: true
Attachment #8387404 -
Flags: feedback?(jmathies)
Attachment #8388701 -
Flags: feedback?(jmathies)
Assignee | ||
Comment 14•11 years ago
|
||
Attachment #8387405 -
Attachment is obsolete: true
Attachment #8387405 -
Flags: feedback?(jmathies)
Attachment #8388705 -
Flags: feedback?(jmathies)
Assignee | ||
Comment 15•11 years ago
|
||
(In reply to Jim Mathies [:jimm] (away 4/1-4/21) from comment #11)
> This is acting kind of strange in text areas -
>
> 1) off an initial drag, I always seem to select from the caret position down
> to the the line below the caret position.
>
> 2) selection seems to jump around selecting larger blocks that I'm aiming
> for.
>
> 3) dragging across the other monocle doesn't reverse selection, it still
> seems to drag the other monocle with it resulting in a single character of
> selection dragging under my finger.
Thanks for checking this out! Yeah as I mentioned textareas have a bunch of problems. I've tried to improve swapping for general text and textareas, but it's still not as precise as I want it to be. I think it would be better to file follow ups to cover every corner case + polish it if required.
Assignee | ||
Comment 16•11 years ago
|
||
Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=66cbf7c9d49f
Reporter | ||
Comment 17•11 years ago
|
||
Comment on attachment 8388696 [details] [diff] [review]
Part 1: Swapping start and end monocles when they overlap
Review of attachment 8388696 [details] [diff] [review]:
-----------------------------------------------------------------
::: browser/metro/base/content/helperui/SelectionHelperUI.js
@@ +137,5 @@
> get dragging() {
> return this._element.customDragger.dragging;
> },
>
> + get restrictedToBounds() {
can you add a comment here explaining what this flag does?
@@ +947,5 @@
>
> + _selectionSwap: function _selectionSwap() {
> + [this.startMark.tag, this.endMark.tag] = [this.endMark.tag,
> + this.startMark.tag];
> + [this._startMark, this._endMark] = [this.endMark, this.startMark];
nice!
Attachment #8388696 -
Flags: feedback?(jmathies) → review+
Reporter | ||
Updated•11 years ago
|
Attachment #8388701 -
Flags: feedback?(jmathies) → review+
Reporter | ||
Comment 18•11 years ago
|
||
Comment on attachment 8388705 [details] [diff] [review]
Part 3: Mochitests for content and chrome input selection.
I got failures in both of these running them locally, but it looks like your test push to try came up green. Have you had any trouble running these locally?
Reporter | ||
Comment 19•11 years ago
|
||
Comment on attachment 8388705 [details] [diff] [review]
Part 3: Mochitests for content and chrome input selection.
Review of attachment 8388705 [details] [diff] [review]:
-----------------------------------------------------------------
::: browser/metro/base/tests/mochitest/browser_selection_inputs.js
@@ +221,5 @@
> + let touchDrag = new TouchDragAndHold();
> + yield touchDrag.start(gWindow, startXPos, startYPos, 320, startYPos);
> +
> + yield waitForCondition(() => getTrimmedSelection(gInput).toString() ==
> + "straight on like", kCommonWaitMs, kCommonPollMs);
here I got "straight on lik" and the test times out.
::: browser/metro/base/tests/mochitest/browser_selection_urlbar.js
@@ +341,5 @@
> +
> + // Place caret to the input start
> + sendTap(window, selectionRectangle.left, selectionRectangle.top);
> + yield waitForCondition(() => !SelectionHelperUI.isSelectionUIVisible &&
> + SelectionHelperUI.isCaretUIVisible);
this timed out for me locally.
Assignee | ||
Comment 20•11 years ago
|
||
Attachment #8388696 -
Attachment is obsolete: true
Attachment #8389118 -
Flags: review+
Assignee | ||
Comment 21•11 years ago
|
||
Attachment #8388705 -
Attachment is obsolete: true
Attachment #8388705 -
Flags: feedback?(jmathies)
Attachment #8389120 -
Flags: review?(jmathies)
Assignee | ||
Comment 22•11 years ago
|
||
Thanks for review!
(In reply to Jim Mathies [:jimm] (away 4/1-4/21) from comment #17)
> Comment on attachment 8388696 [details] [diff] [review]
> Part 1: Swapping start and end monocles when they overlap
>
> Review of attachment 8388696 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: browser/metro/base/content/helperui/SelectionHelperUI.js
> @@ +137,5 @@
> > get dragging() {
> > return this._element.customDragger.dragging;
> > },
> >
> > + get restrictedToBounds() {
>
> can you add a comment here explaining what this flag does?
Done!
(In reply to Jim Mathies [:jimm] (away 4/1-4/21) from comment #19)
> Comment on attachment 8388705 [details] [diff] [review]
> Part 3: Mochitests for content and chrome input selection.
>
> Review of attachment 8388705 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: browser/metro/base/tests/mochitest/browser_selection_inputs.js
> @@ +221,5 @@
> > + let touchDrag = new TouchDragAndHold();
> > + yield touchDrag.start(gWindow, startXPos, startYPos, 320, startYPos);
> > +
> > + yield waitForCondition(() => getTrimmedSelection(gInput).toString() ==
> > + "straight on like", kCommonWaitMs, kCommonPollMs);
>
> here I got "straight on lik" and the test times out.
>
> ::: browser/metro/base/tests/mochitest/browser_selection_urlbar.js
> @@ +341,5 @@
> > +
> > + // Place caret to the input start
> > + sendTap(window, selectionRectangle.left, selectionRectangle.top);
> > + yield waitForCondition(() => !SelectionHelperUI.isSelectionUIVisible &&
> > + SelectionHelperUI.isCaretUIVisible);
>
> this timed out for me locally.
Hmm, didn't have problems on my main Lenovo W530, but just run tests on Surface and noticed the same failures. Looks like touchAndDrag accuracy depends on resolution, so I just simplified tests, but it shouldn't be a problem for the user. But that is strange anyway, I thought setDevPixelEqualToPx() call should have helped.
Pushed updated tests to try: https://tbpl.mozilla.org/?tree=Try&rev=139c9df0e673
Reporter | ||
Updated•11 years ago
|
Attachment #8389120 -
Flags: review?(jmathies) → review+
Assignee | ||
Comment 23•11 years ago
|
||
Try tests have passed: https://tbpl.mozilla.org/?tree=Try&rev=139c9df0e673
Keywords: checkin-needed
Comment 24•11 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/3c9c98992742
https://hg.mozilla.org/integration/fx-team/rev/1b636faa3187
https://hg.mozilla.org/integration/fx-team/rev/b1b516d5c5e6
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [selection] p=5 s=it-30c-29a-28b.3 r=ff30 → [selection] p=5 s=it-30c-29a-28b.3 r=ff30[fixed-in-fx-team]
Comment 25•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3c9c98992742
https://hg.mozilla.org/mozilla-central/rev/1b636faa3187
https://hg.mozilla.org/mozilla-central/rev/b1b516d5c5e6
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [selection] p=5 s=it-30c-29a-28b.3 r=ff30[fixed-in-fx-team] → [selection] p=5 s=it-30c-29a-28b.3 r=ff30
Target Milestone: --- → Firefox 30
Updated•11 years ago
|
Whiteboard: [selection] p=5 s=it-30c-29a-28b.3 r=ff30 → [selection] p=5 s=it-30c-29a-28b.3 r=ff30 [qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•