Specific styling causes graphic corruption when I scroll fake scrollbar on https://vk.com/

VERIFIED FIXED in Firefox 50

Status

()

defect
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: arni2033, Assigned: sotaro)

Tracking

({regression})

48 Branch
mozilla51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 unaffected, firefox48 wontfix, firefox49 wontfix, firefox50 fixed, firefox51 verified)

Details

Attachments

(3 attachments, 5 obsolete attachments)

>>>   My Info:   Win7_64, Nightly 50, 32bit, ID 20160726080520 (2016-07-26)
STR_1:
0. Make sure DPI is set to 1.0
1. Open http://opaliha-o3.org/
2. Scroll the page so that iframe http://vk.com/ was fully visible
3. Move mouse to the center of that iframe, rotate mouse wheel to the bottom several times

AR:  A dark-blue line appears at the right side of scrollbar
ER:  No extra blue lines

Note:  If you can't reproduce, I recommend to test this on "Jeff's machine" mentioned in bug 1234567

This is regression from bug 1245552. Regression range:
> https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d916e452018046a5b2cb7699937e2d40171bf9e4&tochange=85f0d8ad266f3132aa4d5ff7cd23cab41b128af2
Flags: needinfo?(sotaro.ikeda.g)
Correction:
* to test this on "Jeff' machine" mentioned in bug 1287066. It's in "See also" list.
Let me know if you can't reproduce this Sotaro and I can test using the Toronto windows machine for us, including Jeff's machine.
Assignee: nobody → sotaro.ikeda.g
At first I failed to reproduce the problem. Then I just succeeded to reproduce the problem. It is easier to reproduce the problem when HWA is enabled.
Too late for 48 but happy to take a patch in 49
There seems to exist an inconsitency between surface size and maskTransform by changing surface size.
Flags: needinfo?(sotaro.ikeda.g)
attachment 8776454 [details] [diff] [review] addressed problem for me.
Attachment #8776454 - Attachment description: patch - Change surfaceSize calculation → patch - Change mask SurfaceSize calculation
Attachment #8776454 - Attachment description: patch - Change mask SurfaceSize calculation → patch part 1 - Change mask SurfaceSize calculation
Attachment #8776823 - Attachment description: patch - fuzz some tests → patch part2 - fuzz some tests
Attachment #8776823 - Attachment description: patch part2 - fuzz some tests → patch part 2 - fuzz some tests
Attachment #8776823 - Attachment is obsolete: true
Somehow fuzz of clipping-7.html did not work. It seems to be caused by "fuzzy-if(skiaContent,16,27)", if it is removed, the fuzz worked well.
(In reply to Sotaro Ikeda [:sotaro] from comment #12)
> Somehow fuzz of clipping-7.html did not work. It seems to be caused by
> "fuzzy-if(skiaContent,16,27)", if it is removed, the fuzz worked well.

Previous general fuzz setting was overridden by "fuzzy-if(skiaContent,16,27)".
Attachment #8776853 - Attachment is obsolete: true
Attachment #8776454 - Flags: review?(matt.woodrow)
Attachment #8777183 - Flags: review?(matt.woodrow)
Attachment #8776454 - Flags: review?(matt.woodrow) → review+
Attachment #8777183 - Flags: review?(matt.woodrow) → review+
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b940f1bf535
Change mask SurfaceSize calculation r=mattwoodrow
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/242fcdd56995
Backed out changeset 5b940f1bf535 for reftest failures
backed out for reftest failures like https://treeherder.mozilla.org/logviewer.html#?job_id=33297848&repo=mozilla-inbound
Flags: needinfo?(sotaro.ikeda.g)
Needs to add more fuzz to win :(
Flags: needinfo?(sotaro.ikeda.g)
Attachment #8777183 - Attachment is obsolete: true
Attachment #8777716 - Flags: review+
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e8a67e61102
Change mask SurfaceSize calculation r=mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/4e8a67e61102
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Depends on: 1294342
Want to request uplift here? 
Do we think this is a common problem users see?
Flags: needinfo?(sotaro.ikeda.g)
Flags: needinfo?(bgirard)
I don't think mask layers are used that often but I can't say exactly. I think we're using them a bit more recently. I'll let Sotaro make the call.
Flags: needinfo?(bgirard)
backed out:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e91880668542e00140784bc57e747aece36480ae

for a performance regression in bug 1294342.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The change is necessary to prevent oom on android, but it seems to cause performance regression on another platforms.
Flags: needinfo?(sotaro.ikeda.g)
Change mask SurfaceSize calculation when MOZ_GFX_OPTIMIZE_MOBILE is defined.
Attachment #8776454 - Attachment is obsolete: true
Comment on attachment 8781792 [details] [diff] [review]
patch part 1 - Change mask SurfaceSize calculation

:mattwoodrow, can you review the patch again?
Attachment #8781792 - Flags: review?(matt.woodrow)
Attachment #8781792 - Flags: review?(matt.woodrow) → review+
(In reply to Sotaro Ikeda [:sotaro] from comment #29)
> Created attachment 8781792 [details] [diff] [review]
> patch part 1 - Change mask SurfaceSize calculation
> 
> Change mask SurfaceSize calculation when MOZ_GFX_OPTIMIZE_MOBILE is defined.

By attachment 8781792 [details] [diff] [review], alignment change is done only on android. Then, it should not degrade the performance on another platforms.
Rebased.
Attachment #8777716 - Attachment is obsolete: true
Attachment #8782239 - Flags: review+
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/110094e195de
Change mask SurfaceSize calculation r=mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/110094e195de
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Sotaro please request approvals for uplift to 50 and 49.
Flags: needinfo?(sotaro.ikeda.g)
Comment on attachment 8781792 [details] [diff] [review]
patch part 1 - Change mask SurfaceSize calculation

Approval Request Comment
[Feature/regressing bug #]: Bug 1245552
[User impact if declined]: Could cause graphic corruption when mask is used.
[Describe test coverage new/current, TreeHerder]: Locally tested.
[Risks and why]: Low.
[String/UUID change made/needed]: None.
Flags: needinfo?(sotaro.ikeda.g)
Attachment #8781792 - Flags: approval-mozilla-beta?
Attachment #8781792 - Flags: approval-mozilla-aurora?
See Also: → 1299354
Hi arni2033, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(arni2033)
Comment on attachment 8781792 [details] [diff] [review]
patch part 1 - Change mask SurfaceSize calculation

Fixes a recent regression, Aurora50+
Attachment #8781792 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I think here we may let the fix ride with 50. If I'm wrong here and you think the impact on users is large, or on many users, we could still consider uplift. But I would rather not take on more last minute uplifts for 49.
Comment on attachment 8781792 [details] [diff] [review]
patch part 1 - Change mask SurfaceSize calculation

Too late for 49 for uncertain user benefit, we have only 1 week till release.
Attachment #8781792 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
(In reply to Ritu Kothari (:ritu) from comment #38)
> Hi arni2033, could you please verify this issue is fixed as expected on a latest Nightly build?
No bug:   Win7_64, Nightly 51, 32bit, ID 20160913030425 (2016-09-13)

Well, actually that site in iframe (vk.com) changed its styling BEFORE 2016-09-01 (comment 38), so I
couldn't possibly verify this. Now it's pointless to test url from comment 0.
Luckily I found a copy of original page on my hard drive, and now I can verify that this bug doesn't happen. I didn't immediately found the saved page, and also I was busy, so it took me a long time.
Status: RESOLVED → VERIFIED
Flags: needinfo?(arni2033)
(In reply to arni2033 from comment #43)
> (In reply to Ritu Kothari (:ritu) from comment #38)
> > Hi arni2033, could you please verify this issue is fixed as expected on a latest Nightly build?
> No bug:   Win7_64, Nightly 51, 32bit, ID 20160913030425 (2016-09-13)
> 
> Well, actually that site in iframe (vk.com) changed its styling BEFORE
> 2016-09-01 (comment 38), so I
> couldn't possibly verify this. Now it's pointless to test url from comment 0.
> Luckily I found a copy of original page on my hard drive, and now I can
> verify that this bug doesn't happen. I didn't immediately found the saved
> page, and also I was busy, so it took me a long time.

Awesome! Thank you so much for your due diligence. :)
See Also: → 1327329
You need to log in before you can comment on or make changes to this bug.