1278021 - Yelp gallery broken in Firefox 48

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Kevin Brosnan [Ex-Mozilla]]

* Open http://www.yelp.com/biz_photos/queen-house-mountain-view
* click on one of the images in the gallery

Expected: image slide show appears
Actual: nothing happens on click

 6:25.34 INFO: Last good revision: dd5753b1f0143e0d68d343070a62ad47e5b1ed54
 6:25.34 INFO: First bad revision: 849e85ce8561cac62c4fe5a3ebb505ca0c238a93
 6:25.34 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=dd5753b1f0143e0d68d343070a
62ad47e5b1ed54&tochange=849e85ce8561cac62c4fe5a3ebb505ca0c238a93

 6:25.65 INFO: Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1250718

Matt looks like you regressed this.

Comment 1

[Alice0775 White]

Attachment: testcase#1

Blue rectangle should be clickable.

Comment 2

[Matt Woodrow (:mattwoodrow)]

Attachment: bug1278021

Comment 3

[Thinker Li [:sinker]]

Comment on attachment 8760531 [details] [diff] [review]
bug1278021

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

::: layout/generic/nsFrame.cpp
@@ +2136,4 @@
>      transformFrame = aItem->Frame();
> +  } else if (aItem->GetType() == nsDisplayItem::TYPE_OPACITY) {
> +    transformFrame = aItem->Frame();
> +    if (!transformFrame->IsTransformed()) {

It implies not only no Extend3DContext(), but also having no transform property at leaves and more.  It's intention is not clear literally.  Maybe we should clarify it at the comment of IsTransformed().

Comment 4

[Thinker Li [:sinker]]

Attachment: wording-istransformed.diff

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7bae0fb42b79
Don't treat nsDisplayOpacity as participating in a preserve-3d context if it is an untransformed leaf. r=thinker
https://hg.mozilla.org/integration/mozilla-inbound/rev/84811a596e32
Clarify what nsIFrame::IsTransformed() does. r=mattwoodrow

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7bae0fb42b79
https://hg.mozilla.org/mozilla-central/rev/84811a596e32

Comment 7

[Marcia Knous [:marcia]]

Tracking 48/49+ for this site compat regression.

Comment 8

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Should this be backported?

Comment 9

[Gerry Chang [:gchang]]

Hi Jet, 
Do you think you can find someone for the patch to be backported in 48 and 49?

Comment 10

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8760531 [details] [diff] [review]
bug1278021

Approval Request Comment
[Feature/regressing bug #]: Bug 1250718
[User impact if declined]: Hit testing with opacity and preserve-3d is sometimes broken
[Describe test coverage new/current, TreeHerder]: Manually tested, mochitest added
[Risks and why]: Low risk.
[String/UUID change made/needed]: None

Comment 11

[Gerry Chang [:gchang]]

Comment on attachment 8760531 [details] [diff] [review]
bug1278021

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

This patch fixes a regression. Take it in 48 beta 6 and aurora.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/fe119f3a7405
https://hg.mozilla.org/releases/mozilla-aurora/rev/c25ec152b6ef

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f30baf0d1daa
https://hg.mozilla.org/releases/mozilla-beta/rev/b9e992567411

Comment 14

[julesmyers2011]

Image gallery pops up just fine in Firefox 48.0b6
OS: OS X Yosemite 10.10.2

Comment 15

[Mihai Boldan, Desktop QA [:mboldan]]

Thanks julesmyers2011 for testing.
I managed to reproduce this issue on Firefox 48.0a1 (2016-04-03) and on WIndows 10 x64.
The issue is no longer reproducible on Firefox 48.0b6, Firefox 49.0a2 (2016-07-10) and on Firefox 50.0a1 (2016-07-10).
The test were performed under Windows 10 x64, Ubuntu 16.04 x64, Mac OS X 10.11.1 and using STR from Comment 0 and Testcase#1.