Bug 19125

Summary: [Qt] add functions to get/set the zoom level
Product: WebKit Reporter: Siraj razick <siraj>
Component: WebKit QtAssignee: Simon Hausmann <hausmann>
Status: RESOLVED FIXED    
Severity: Normal CC: alp, hausmann, pierre-luc.beaudoin
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
A Patch that adds functions to get/set the zoom level to QtWebKit
none
Add functions to get/set the zoom level
hausmann: review-
Add functions to get/set the zoom level V2.
none
Zoom factor patch v3
zecke: review+
Zoom factor patch v4 zecke: review+

Description Siraj razick 2008-05-19 11:15:13 PDT
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.
Comment 1 Siraj razick 2008-05-21 07:02:14 PDT
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
Comment 2 Siraj razick 2008-05-21 10:49:26 PDT
Created attachment 21271 [details]
Add functions to get/set the zoom level
Comment 3 Pierre-Luc Beaudoin 2008-05-21 11:28:46 PDT
Comment on attachment 21271 [details]
Add functions to get/set the zoom level

Actually you have to use "review?" first :)
Comment 4 Simon Hausmann 2008-06-06 01:37:37 PDT
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 :)
Comment 5 Siraj razick 2008-06-12 09:27:42 PDT
(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.
Comment 6 Siraj razick 2008-06-24 09:42:13 PDT
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.
Comment 7 Eric Seidel (no email) 2008-07-24 11:57:39 PDT
Comment on attachment 21911 [details]
Add functions to get/set the zoom level V2. 

Looks fine to me.
Comment 8 Simon Hausmann 2008-07-26 05:56:17 PDT
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.
Comment 9 Mark Rowe (bdash) 2008-07-26 22:34:19 PDT
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.
Comment 10 Holger Freyther 2008-08-02 14:27:09 PDT
Comment on attachment 22494 [details]
Zoom factor patch v3

Look's good. What made you pick zoomTextOnly over zoomOnlyText?
Comment 11 Simon Hausmann 2008-08-05 08:28:17 PDT
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.
Comment 12 Holger Freyther 2008-08-11 18:13:38 PDT
Comment on attachment 22655 [details]
Zoom factor patch v4

- API is okay
- WebCore::Settings change is fine
Comment 13 Simon Hausmann 2008-08-13 06:43:53 PDT
Landed in r35716