Bug 19125 – [Qt] add functions to get/set the zoom level

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Siraj razick]

Qt WebKit currently seems to have text only zoom. I'm going to write a patch to expose an API function to zoom all the contents, including text and images.

[Siraj razick]

Created attachment 21267 [details]
A Patch that adds functions to get/set the zoom level to QtWebKit 

A Patch that adds functions to get/set the zoom level to QtWebKit API

[Siraj razick]

Created attachment 21271 [details]
Add functions to get/set the zoom level

[Pierre-Luc Beaudoin]

Comment on attachment 21271 [details]
Add functions to get/set the zoom level

Actually you have to use "review?" first :)

[Simon Hausmann]

Comment on attachment 21271 [details]
Add functions to get/set the zoom level

In principle I agree with the patch, but I see two problems:

1) Documentation is missing for this property.

2) The return value of Frame::zoomFactor() can be the text size multiplier or the zoom factor. I think we should use the newly added pageZoomFactor() and textZoomFactor() getters. But I'm not sure what to do about the two conflicting and symmetry. Either way we have to document the interaction of the two properties or how they exclude each other.

Another option is to add support for both...

So r- because this needs more work, but don't let that discourage you! It's great that you've started on this missing feature :)

[Siraj razick]

(In reply to comment #4) 
Sorry for the delay, I was stuck with some work. I'll re-prepare a patch along the lines you suggested. I really appreciate the review, it's all part of the learning curve.

[Siraj razick]

Created attachment 21911 [details]
Add functions to get/set the zoom level V2. 

Hi,
Sorry for the delay. Here is the patch which adds page zoom and text zoom API properties. I have added the required documentation and also this patch is using   the new pageZoomFactor()/textZoomFactor() functions from the webCore, as suggested by Simon.

[Eric Seidel (no email)]

Comment on attachment 21911 [details]
Add functions to get/set the zoom level V2. 

Looks fine to me.

[Simon Hausmann]

Created attachment 22494 [details]
Zoom factor patch v3

This patch is based on Siraj's patch but proposes a slightly different API. I think zoomFactor is more consistent with the rest of Qt and the boolean zoomTextOnly property gives clarity over the meaning of the zoomFactor.

[Mark Rowe (bdash)]

Comment on attachment 21911 [details]
Add functions to get/set the zoom level V2. 

Clearing review flag since Comment #8 suggests that the  API is still under discussion.

[Holger Freyther]

Comment on attachment 22494 [details]
Zoom factor patch v3

Look's good. What made you pick zoomTextOnly over zoomOnlyText?

[Simon Hausmann]

Created attachment 22655 [details]
Zoom factor patch v4

This is a new revision of the previous patch that maps the implementation and moves zoomTextOnly into QWebSettings.

After a discussion with the doc team we concluded that having 'Only' at the end is more consistent with the rest of Qt, which has for example setReadOnly or setForwardOnly.

[Holger Freyther]

Comment on attachment 22655 [details]
Zoom factor patch v4

- API is okay
- WebCore::Settings change is fine

[Simon Hausmann]

Landed in r35716