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

[bugzilla-daemon at webkit.org]

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


Kenneth Rohde Christiansen <kenneth.christiansen at openbossa.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kenneth.christiansen at openbo
                   |                            |ssa.org




--- Comment #5 from Kenneth Rohde Christiansen <kenneth.christiansen at openbossa.org>  2009-09-15 17:20:20 PDT ---
(In reply to comment #4)
> Created an attachment (id=39619)
 --> (https://bugs.webkit.org/attachment.cgi?id=39619) [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.

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