Created attachment 808101 [details] [diff] [review] patch nsLayoutUtils::UpdateImageVisibilityForFrame expands the image rect to the scrollport of the scrollframe for each scroll frame it encounters. This is way too much. The rect of the image may be scrolled out of view in the scroll frame. Using a rect that is scrolled out of view for further visibility calculations seems wrong. Instead pretend the rect was scrolled into view.
This makes sense for a small image (relative the scroll port), but it seems overly pessimistic for an image that's larger than the scroll port. I think we want to clip the result to the scroll port after the lines you added. (which, given that x,y is now inside the scroll port with the lines you added, boils down to std::max of the two widths/heights) Also, transformedRect.x += scrollPort.x - transformedRect.x; is the same as: transformedRect.x = scrollPort.x;
Er, std::min that is... bah, reviewing while having morning coffee ;-)
(In reply to Mats Palmgren (:mats) from comment #1) > I think we want to clip the result to the > scroll port after the lines you added. Yes of course. This was what I always intended to do but forget to add after I had the shifting part done.
Created attachment 808202 [details] [diff] [review] patch v2
Comment on attachment 808202 [details] [diff] [review] patch v2 OK, looks good. r=mats
Comment on attachment 808202 [details] [diff] [review] patch v2 [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 847223 User impact if declined: this patch just fixes the code that was added in bug 847223 to determine if a image is visible or not. it stops images from being considered visible or nearly visible when they are not. taking this patch means we only have one version of this code around and never ship the sub-optimal code. Testing completed (on m-c, etc.): been on m-c for a while now, no problems found Risk to taking this patch (and alternatives if risky): not risky String or IDL/UUID changes made by this patch: none
We probably don't need to bother triaging this for koi at this point, given that this is already landed on aurora per comment 10.