Closed Bug 1171731 Opened 5 years ago Closed 5 years ago

Zoomed View unexpectedly activates when textSize=0

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 41
Tracking Status
firefox41 --- fixed

People

(Reporter: mcomella, Assigned: domivinc)

References

()

Details

Attachments

(2 files)

* Go to http://www.wsj.com/articles/in-hong-kong-the-apartments-are-fit-for-a-mosquito-1433237582?mod=e2fb
* Scroll down to the image carousel
* Hit one of the cycle buttons

Expected: The image changes
Actual: The zoomed view appears

The hit target seems large enough that I wouldn't expect this to happen - any ideas, Dominique?

By the way, you can use remote debugging on your Android device to try to figure out what's going on in the site's mobile HTML.
Hi Michael, the html of the button is the following:

<button type="button" data-role="none" class="slick-next" aria-label="next" style="display: block;">Next</button>

To avoid to display the text "Next" with the background picture of the button, the font size has been set to 0: "font-size: 0;" in the class "slick-next".

The cluster detection process detects that the button text is too small to be readable (0 size), so the zoomed view shows up.

One possible fix is to consider that all the elements using a 0 font size should be ignored in the cluster detection process.
Michael, I added the test for elements with font size set to 0.
It seems to fix the issue on my device.

Kats should probably review this change in PositionedEventTargeting.cpp ?
Flags: needinfo?(michael.l.comella)
Attachment #8615995 - Flags: review?(bugmail.mozilla)
Dominique, the patch worked for me. :)
Flags: needinfo?(michael.l.comella)
Comment on attachment 8615995 [details] [diff] [review]
patch-05062015 2-Bug_1171731___Ignore_elements_with_0_font_size_in_cluster_detection__r_kats.patch

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

::: layout/base/PositionedEventTargeting.cpp
@@ +416,5 @@
>    nsRefPtr<nsFontMetrics> fm;
>    nsLayoutUtils::GetFontMetricsForFrame(aFrame, getter_AddRefs(fm),
>      nsLayoutUtils::FontSizeInflationFor(aFrame));
>    if (fm) {
> +    if ((fm->EmHeight() > 0) && // See bug 1171731

This is fine, but you can also combine it with the enclosing if statement:

if (fm && fm->EmHeight() > 0 &&
    pc->AppUnitsToGfxUnits.....) {
  return false;
}
Attachment #8615995 - Flags: review?(bugmail.mozilla) → review+
Summary: Zoomed View unexpectedly activates on image carousel button → Zoomed View unexpectedly activates when textSize=0
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b4f355ca322f
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
You need to log in before you can comment on or make changes to this bug.