Closed Bug 912700 Opened 6 years ago Closed 5 years ago

Audit all uses of FrameMetrics::mViewport for correctness and consistency

Categories

(Core :: Panning and Zooming, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(1 file)

Along with mCompositionBounds, mViewport is the other poorly-defined field in FrameMetrics. Actually, it's defined reasonably enough, but uses of it are inconsistent. mViewport is supposed to be the CSS viewport of the document, which means that (a) it should only be valid for the layers that correspond to documents and (b) could be affected by meta viewport tags on the document in question, so may not be the same as the visible rect or composition bounds of the frame.

Looking at the uses of mViewport [1] shows that a bunch of pieces of code conflate this field with the composition bounds and will need to be fixed up. 

[1] http://dxr.mozilla.org/mozilla-central/search?q=%2Bvar-ref%3Amozilla%3A%3Alayers%3A%3AFrameMetrics%3A%3AmViewport
Component: Graphics: Layers → Panning and Zooming
Bug 1056427 removes some uses of mViewport that appear totally bogus.
Depends on: 1056427
Attached patch PatchSplinter Review
And I audited all the rest and they look good. Also wrapped them in a getter/setter while I was at it.
Assignee: nobody → bugmail.mozilla
Attachment #8476667 - Flags: review?(botond)
Attachment #8476667 - Flags: review?(botond) → review+
Was there a reason for that try push?
Flags: needinfo?(mmc)
Sorry about that, I had just pulled and was trying to test the try chooser extension so your rev was at the top of my queue.
Flags: needinfo?(mmc)
https://hg.mozilla.org/mozilla-central/rev/95a32b449dc5
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.