932783 - Selection monocles don't update position with apzc scroll

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Pan and Zoom, defect)

Description

[Jim Mathies [:jimm]]

str:

1) press hold to select some text
2) touch scroll the page

result: scroll monocles remain in their original position

Comment 1

[Jim Mathies [:jimm]]

Attachment: patch

Comment 2

[Jim Mathies [:jimm]]

Attachment: patch

What this does is keeps selection active as long as selection remains visible. If not, it clears. It would be nice if we could keep selection going even if it's off screen, but that's trickier. I think that can wait until we get selection down into dom. (which ehsan is supposedly working on!)

Comment 3

[Jim Mathies [:jimm]]

Comment on attachment 824798 [details] [diff] [review]
patch

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

::: browser/metro/base/content/helperui/SelectionHelperUI.js
@@ +359,5 @@
> +                               event.clientX, event.clientY);
> +        break;
> +
> +      case "apzc-handle-pan-begin":
> +        Util.dumpLn("apzc-handle-pan-begin");

oops, nixed locally.

Comment 4

[Matt Brubeck (:mbrubeck)]

Comment on attachment 824798 [details] [diff] [review]
patch

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

r=mbrubeck, though with a question about a possible change/followup:

::: browser/metro/base/content/helperui/SelectionHelperUI.js
@@ +350,5 @@
>     * Observers
>     */
>  
>    observe: function (aSubject, aTopic, aData) {
> +    let self = SelectionHelperUI;

I think we can remove this line and just use "this".

@@ +903,5 @@
>  
> +  _checkMonocleVisibility: function(aX, aY) {
> +    if (aX < 0 || aY < 0 ||
> +        aX > ContentAreaObserver.viewableWidth ||
> +        aY > ContentAreaObserver.viewableHeight) {

This makes it impossible to select something larger than the screen, doesn't it...

How hard would it be to just hide the monocles while they're offscreen, but keep the edit session "open" (so the monocles could reappear if you scroll back to them)?

Comment 5

[Jim Mathies [:jimm]]

(In reply to Matt Brubeck (:mbrubeck) from comment #4)
> How hard would it be to just hide the monocles while they're offscreen, but
> keep the edit session "open" (so the monocles could reappear if you scroll
> back to them)?

Keeping the selection handler active all the time is untested territory. It's a change I've been meaning to make, it just needs a lot of testing to see what breaks.

Comment 6

[Jim Mathies [:jimm]]

https://hg.mozilla.org/integration/fx-team/rev/3bde0e601ab8

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out alongside bug 937750 for being a possible cause for the bustage of mochitest-metro:  https://hg.mozilla.org/integration/fx-team/rev/b43a066f1619

https://tbpl.mozilla.org/php/getParsedLog.php?id=30726849&tree=Fx-Team

Comment 8

[Jim Mathies [:jimm]]

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d2f9d2fcc012