[Webkit-unassigned] [Bug 28781] Add QWebFrame::renderElement to API

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=28781





--- Comment #8 from Viatcheslav Ostapenko <ostapenko.viatcheslav at nokia.com>  2009-09-16 11:22:38 PDT ---
(In reply to comment #5)
> (In reply to comment #4)
> > Created an attachment (id=39619)
 --> (https://bugs.webkit.org/attachment.cgi?id=39619) [details] [details]
> > Updated QWebElement::render implementation with API autotest
> 
> Cool feature you are implementing and nice with the test.
> 
> I didn't look at the patch itself, but the coding style it at least not right,
> so please fix that first :-)
> 
> 
> >+    Frame* cFrame = doc->frame();
> 
> Why cFrame? what does the c stand for? 
> 
> >+    IntRect pRect = e->getRect();
> 
> name here with pRect!
> 
> This is not WebKit coding style, you need to write out the name, or just use
> 'frame' and 'rect'
> 
> >+    WebCore::RenderObject* WebKitRenderer = e->renderer();
> 
> This one is even worse. What is a WebKit Renderer? it is actually a
> RenderObject. Plus, the variable should always start with a lowercase letter.
> RenderTheme just uses o for RenderObjects, so
> 
> RenderObject* o = e->renderer() would be OK I guess. Or spell it out
> renderObject.
> 
> >+        Image* img = toRenderImage(WebKitRenderer)->cachedImage()->image();
> 
> I would spell out img as image
> 
> >+    WebCore::RenderObject::PaintInfo cPaintInfo(&context, pRect, PaintPhaseBlockBackground, false, 0, 0);
> 
> In Qt, we try not to pass booleans without comments when it is not obvious what
> they mean.
> 
> Examples:
> 
> setEnabled(true) is right
> doSomething(rect, true) is wrong
> doSomething(rect, /* clear cache */ true);
> 
> That was the code is easier to read and understand afterward.

Ok, I'll fix that.

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