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

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

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

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Bug 1056427 removes some uses of mViewport that appear totally bogus.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

And I audited all the rest and they look good. Also wrapped them in a getter/setter while I was at it.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/95a32b449dc5

Comment 4

[[:mmc] Monica Chew (no longer reading bugmail)]

https://tbpl.mozilla.org/?tree=Try&rev=95a32b449dc5

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Was there a reason for that try push?

Comment 6

[[:mmc] Monica Chew (no longer reading bugmail)]

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.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/95a32b449dc5