Closed
Bug 1034179
Opened 10 years ago
Closed 10 years ago
GetOrMaybeCreateDisplayport uses the wrong composition bounds, results in a bad displayport
Categories
(Core :: Panning and Zooming, defect)
Tracking
()
People
(Reporter: kats, Assigned: kats)
References
Details
Attachments
(2 files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
tnikkel
:
review+
|
Details | Diff | Splinter Review |
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 | ||
Updated•10 years ago
|
Assignee: nobody → bugmail.mozilla
Comment 1•10 years ago
|
||
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.
Assignee | ||
Comment 2•10 years ago
|
||
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
Assignee | ||
Comment 3•10 years ago
|
||
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)
Updated•10 years ago
|
Attachment #8450449 -
Flags: review?(tnikkel) → review+
Assignee | ||
Comment 5•10 years ago
|
||
Thanks for the quick review!
https://hg.mozilla.org/integration/mozilla-inbound/rev/87ffae4aea2b
Comment 6•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Assignee | ||
Updated•10 years ago
|
Updated•10 years ago
|
blocking-b2g: 2.0? → 2.0+
Comment 7•10 years ago
|
||
NO_UPLIFT until sufficiently baked on trunk per discussion with kats.
Whiteboard: [NO_UPLIFT]
Comment 8•10 years ago
|
||
(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).
Assignee | ||
Comment 9•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•