1141855 - Don't use APZ for selection carets on Desktop

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

Selection caret tests do not work with Desktop APZ. I'm guessing the "long tap" stuff isn't hooked up. This isn't very important for desktop APZ so I'd like to just change selection caret APZ to be mobile-only.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: patch

Comment 2

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

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

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

I guess you bump into some marionette test failures after enabling APZ on desktop. This patch looks good to me.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

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

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

What about touch support on desktop? Presumably we'll want to use this SelectionCaret when we support that, do you have a plan to deal with that?

::: layout/base/SelectionCarets.cpp
@@ +114,5 @@
>    }
>  
> +#if defined(MOZ_WIDGET_GONK) || defined(MOZ_WIDGET_ANDROID)
> +  docShell->GetAsyncPanZoomEnabled(&mUseAsyncPanZoom);
> +  mUseAsyncPanZoom = mUseAsyncPanZoom && gfxPrefs::AsyncPanZoomEnabled();

docShell->GetAsyncPanZoomEnabled is redundant here, it just returns the value of the pref which you're getting via gfxPrefs anyway. You can just remove the call to docshell.

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #3)
> What about touch support on desktop? Presumably we'll want to use this
> SelectionCaret when we support that, do you have a plan to deal with that?

Sure, when that time comes, right now I just don't want to block desktop APZ on this.

> > +#if defined(MOZ_WIDGET_GONK) || defined(MOZ_WIDGET_ANDROID)
> > +  docShell->GetAsyncPanZoomEnabled(&mUseAsyncPanZoom);
> > +  mUseAsyncPanZoom = mUseAsyncPanZoom && gfxPrefs::AsyncPanZoomEnabled();
> 
> docShell->GetAsyncPanZoomEnabled is redundant here, it just returns the
> value of the pref which you're getting via gfxPrefs anyway. You can just
> remove the call to docshell.

Ok.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

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

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

In that case just make it MOZ_WIDGET_GONK. Fennec doesn't use this code at all right now either, and just having MOZ_WIDGET_GONK will make it clearer that it's only used on B2G.

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

https://hg.mozilla.org/integration/mozilla-inbound/rev/de19c60e56bd

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

err, landed the wrong patch. here:

https://hg.mozilla.org/integration/mozilla-inbound/rev/6bd5b08c6ff4

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6bd5b08c6ff4