Closed Bug 932783 Opened 11 years ago Closed 11 years ago

Selection monocles don't update position with apzc scroll

Categories

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

x86_64
Windows 8.1
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 28

People

(Reporter: jimm, Assigned: jimm)

References

Details

(Whiteboard: [release28])

Attachments

(1 file, 1 obsolete file)

str:

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

result: scroll monocles remain in their original position
Assignee: nobody → jmathies
Attached patch patch (obsolete) — Splinter Review
Attached patch patchSplinter Review
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!)
Attachment #824795 - Attachment is obsolete: true
Attachment #824798 - Flags: review?(mbrubeck)
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 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)?
Attachment #824798 - Flags: review?(mbrubeck) → review+
(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.
Whiteboard: [block28]
Whiteboard: [block28] → [release28]
https://hg.mozilla.org/mozilla-central/rev/d2f9d2fcc012
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
Depends on: 943071
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: