1184762 - Magnifying glass pops up on Mozilla SSO

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

This is a mobile site with big touch areas, there really shouldn't be any reason for the magnifying glass to show up. And when it does, it is just getting in the way.

Comment 1

[Dominique Vincent [:domivinc]]

Please, could you give the url of the page with the issue? 
And could you post the values used in about:config for: 
ui.mouse.radius.* 
ui.touch.radius.*

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

http://mozilla.okta.com/
all the ui.*.radius.* prefs are their default values

Comment 3

[Dominique Vincent [:domivinc]]

The issue comes from the implementation of Bug 1180811. 
It occurs every time a tap occurs on a clickable element.
It's not specific to your page.
I'm going to fix it.

Comment 4

[Dominique Vincent [:domivinc]]

Attachment: patch-17072015 1-Bug_1184762___Check_visibility_before_closing_with_animation__r_mcomella.patch

I added a test before closing the zoomed view with animation. The animation should occur only if the zoomed view is visible.

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8635711 [details] [diff] [review]
patch-17072015 1-Bug_1184762___Check_visibility_before_closing_with_animation__r_mcomella.patch

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

lgtm, assuming comment 0 doesn't mean the magnifying glass pops up and stays there, but rather the magnifying glass appears and animates out of the frame on click.

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

You should check out the review in bug 1184912 before landing because we might want to consolidate the efforts around where to check for visibility.

Comment 7

[Dominique Vincent [:domivinc]]

Attachment: patch-20072015 1-Bug_1184762___Check_visibility_before_closing_with_animation__r_mcomella.patch

Test of visibility has been moved in stopZoomDisplay method in order to simplify the implementation of bug 1184912.

Comment 10

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8637136 [details] [diff] [review]
patch-20072015 1-Bug_1184762___Check_visibility_before_closing_with_animation__r_mcomella.patch

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

::: mobile/android/base/ZoomedView.java
@@ +501,5 @@
>          moveUsingGeckoPosition(leftFromGecko, topFromGecko);
>      }
>  
>      private void stopZoomDisplay(boolean withAnimation) {
> +        if (getVisibility() == View.VISIBLE) {

nit: I prefer early error-check returns over placing the whole code block in an if statement - I find it a bit easier to reason about. So...

if (getVisibility() != View.VISIBLE) {
  return;
}

// More code...

Comment 11

[Dominique Vincent [:domivinc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=779a5bd0de2a

Comment 12

[Pulsebot]

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

Comment 13

[Ryan VanderMeulen [:RyanVM]]

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