GetOrMaybeCreateDisplayport uses the wrong composition bounds, results in a bad displayport

RESOLVED FIXED in Firefox 32, Firefox OS v2.0

Status

()

Core
Panning and Zooming
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: kats, Assigned: kats)

Tracking

unspecified
mozilla33
All
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.0+, firefox30 wontfix, firefox31 wontfix, firefox32 fixed, firefox33 fixed, b2g-v1.3 unaffected, b2g-v1.4 wontfix, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

Attachments

(2 attachments)

Created attachment 8450383 [details]
Test page

STR:

1. In the B2G browser (Flame running 2.1/master) load the attached test page (taken from bug 1033383)
2. Tap on the text to put focus in the contenteditable field
3. Hit enter twice

At this point, the contenteditable field becomes scrollable, and GetOrMaybeCreateDisplayport is called for it. This function generates a FrameMetrics and then computes a displayport for the scroll frame. The FrameMetrics has an incorrect composition bounds (compared to what gets generated later in RecordFrameMetrics for the same element), and this results in a display port bottom margin that is negative (-21.5510 pixels when I did this). This is wrong, because it leaves the critical displayport not covering the user-visible area. This later results in checkerboarding or low-res painting when it shouldn't.

I'm not sure if GetOrMaybeCreateDisplayport should even be getting called for this element here; from what I remember that should only be happening on initial page load and not when elements are mutated. If the call is OK, then the composition bounds need to be fixed. The element has a height of 100 CSSPixels, which is 150 LayoutDevicePixels, and 49 LayerPixels. RecordFrameMetrics correctly sets a composition bounds that has height 49, but this code computes a composition bounds with height 150, so it's not taking the resolution factor into account properly.
Assignee: nobody → bugmail.mozilla
Blocks: 982141
Every time content paints the first scrollable element we encounter will get a display port via GetOrMaybeCreateDisplayport, so that sounds expected based on how it is currently implemented. Although perhaps we might want to make that more restrictive based on loading of the page like you say.
This bug was introduced in bug 982141, so it's in Gecko 30 and up.
status-b2g-v1.3: --- → unaffected
status-b2g-v1.4: --- → affected
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → affected
status-firefox30: --- → affected
status-firefox31: --- → affected
status-firefox32: --- → affected
status-firefox33: --- → affected
Created attachment 8450449 [details] [diff] [review]
Fix resolution computation

In both RecordFrameMetrics (which this code is mostly a copy of) and in GetDisplayPortFromMarginsData we only use the local presshell's resolution if we're dealing with the root element. This change fixes the problem I was seeing.
Attachment #8450449 - Flags: review?(tnikkel)
Blocks a blocker, nominating for 2.0+
blocking-b2g: --- → 2.0?
No longer blocks: 1033383
Attachment #8450449 - Flags: review?(tnikkel) → review+
Blocks: 1034258
https://hg.mozilla.org/mozilla-central/rev/87ffae4aea2b
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
status-b2g-v1.4: affected → wontfix
status-b2g-v2.1: affected → fixed
status-firefox30: affected → wontfix
status-firefox31: affected → wontfix
status-firefox33: affected → fixed

Updated

4 years ago
blocking-b2g: 2.0? → 2.0+
NO_UPLIFT until sufficiently baked on trunk per discussion with kats.
Whiteboard: [NO_UPLIFT]
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #0)
> I'm not sure if GetOrMaybeCreateDisplayport should even be getting called
> for this element here; from what I remember that should only be happening on
> initial page load and not when elements are mutated.

Our initial approach in bug 982141 was to only call GetOrMaybeCreateDisplayPort on first paint, but we ran into cases such as the Contacts app, where the contacts list is empty (and thus not scrollable) when the page loads, but is populated with contacts (and thus made scrollable) later, so we had to change our approach to call GetOrMaybeCreateDisplayPort on every paint (see https://bugzilla.mozilla.org/show_bug.cgi?id=982141#c15).
https://hg.mozilla.org/releases/mozilla-aurora/rev/b8dfb6b7b99a
status-b2g-v2.0: affected → fixed
status-firefox32: affected → fixed
Whiteboard: [NO_UPLIFT]
You need to log in before you can comment on or make changes to this bug.