1170317 - Audit callers of nsPresContext::GetFullZoom() to see which really want the adjusted full zoom

Status: NEW

(Core :: Layout, defect)

Description

[Botond Ballo [:botond]]

This is a follow-up to bug 1166382, where we discovered that nsPresContext::GetFullZoom() and nsDeviceContext::GetFullZoom() can return different values because the latter returns an adjusted version of the full zoom that makes the number of app units per device pixel an integer.

Some callers of nsPresContext::GetFullZoom() also want the adjusted full zoom instead of the original one; we should go through them and update them accordingly.

Comment 1

[Botond Ballo [:botond]]

Here are the callers, and my guesses as to which one wants which kind of zoom:

nsDocument::TransferZoomLevels           original zoom (the destination will re-adjust it)
nsWindowWatcher::SizeOpenedDocShellItem  adjusted zoom (used for calculations)
AccessibleCaret::GetZoomLevel            adjusted zoom (used for calculations)
nsDocumentViewer::GetFullZoom            original zoom (this is the one used by the UI)
all nsSVGOuterSVGFrame callers           adjusted zoom (used for calculations)

Timothy, do these sound right to you?

Comment 2

[Timothy Nikkel (:tnikkel)]

(In reply to Botond Ballo [:botond] from comment #1)
> nsDocument::TransferZoomLevels           original zoom (the destination will
> re-adjust it)

I'm not sure if this one makes a difference, the adjustment the destination does should be the same. But I think it would be better to use the adjusted ratio (the one that is closest to how we calculate and paint things) in as many places as possible, and only use the original ratio where it is explicitly needed.

> nsDocumentViewer::GetFullZoom            original zoom (this is the one used
> by the UI)

Some of the callers of this function want the original zoom, but do they all? I'm not sure.

Comment 3

[Botond Ballo [:botond]]

(In reply to Timothy Nikkel (:tn) from comment #2)
> (In reply to Botond Ballo [:botond] from comment #1)
> > nsDocument::TransferZoomLevels           original zoom (the destination will
> > re-adjust it)
> 
> I'm not sure if this one makes a difference, the adjustment the destination
> does should be the same. But I think it would be better to use the adjusted
> ratio (the one that is closest to how we calculate and paint things) in as
> many places as possible, and only use the original ratio where it is
> explicitly needed.

The destination is another PresContext, though. If we use the adjusted zoom, the original zoom gets lost, and then if the caller of PresContext::GetFullZoom() that expects the original zoom calls it on the destination, it might get the wrong result.

> > nsDocumentViewer::GetFullZoom            original zoom (this is the one used
> > by the UI)
> 
> Some of the callers of this function want the original zoom, but do they
> all? I'm not sure.

You're right, we should go deeper and look at the callers of this function in turn. Note that this function implements the property nsIContentViewer.fullZoom, so we're going to have to go through the JS callers as well...