|Summary:||[Qt] add functions to get/set the zoom level|
|Product:||WebKit||Reporter:||Siraj razick <siraj>|
|Component:||WebKit Qt||Assignee:||Simon Hausmann <hausmann>|
|Severity:||Normal||CC:||alp, hausmann, pierre-luc.beaudoin|
|Version:||528+ (Nightly build)|
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