[Webkit-unassigned] [Bug 14998] Full page zooming support

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=14998





------- Comment #6 from freyther at handhelds.org  2007-08-24 02:06 PDT -------
Some comments on the patch itself:

+    IntSize size(rect.width(), rect.height());
+    size = i.context->translateSize(size);

=>  IntSize size = i.context->translateSize(rect.size())

+    // Is this optimal?
+    FrameView* frameView = static_cast<const FrameView*>(this);
+    Page* page = frameView->frame() ? frameView->frame()->page() : 0;
+
+    double scaleFactor = page->chrome()->client()->scaleFactor();

a) double -> float -> double? is that what we want? I think not. Even if I
mentioned ChromeCLient::scaleFactor we should not use it here.
b) I wonder why the compiler allows you to do the cast. Probably because this
is not const in this method but then remove const from the cast
c) if you cast in ScrollView to FrameView, either add an ASSERT(isFrameView())
or a if whatever is more appropriate
d) You might want to access the PrivateData of the WebKitGtkPage to get the
scale factor. You can do so by using the Widget::containingWindow().


+static void transform_coordinates_int(GtkWidget* widget, bool inverted, int
*x, int *y)
+static void transform_coordinates_double(GtkWidget* widget, bool inverted,
double *x, double *y)

A small violation of the CodingStyle, You place the '*' (asterisk) at the wrong
place.


+void webkit_gtk_page_set_scale_factor(WebKitGtkPage* page, double factor)
+{
+    WebKitGtkPagePrivate* pageData = WEBKIT_GTK_PAGE_GET_PRIVATE(page);
+
+    pageData->scaleFactor = factor;
+    // TODO: redraw page with new scale factor

a {ScrollView,FrameView}::update() should do the trick. It should be enough to
use the FrameView of the mainFrame.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.