[webkit-reviews] review denied: [Bug 55455] [EFL] HTML saving feature : [Attachment 84208] html saving feature

[bugzilla-daemon at webkit.org]

Antonio Gomes <tonikitoo at webkit.org> has denied Grzegorz
<g.czajkowski at samsung.com>'s request for review:
Bug 55455: [EFL] HTML saving feature
https://bugs.webkit.org/show_bug.cgi?id=55455

Attachment 84208: html saving feature
https://bugs.webkit.org/attachment.cgi?id=84208&action=review

------- Additional Comments from Antonio Gomes <tonikitoo at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=84208&action=review

A bunch of style to fix. please resubmit with these fixed.

> Source/WebKit/efl/ChangeLog:8
> +	   Saves a html source into file given by parameter.

Lets make this change log more descriptive, like adding the TODO's, etc.

> Source/WebKit/efl/ewk/ewk_frame.cpp:2026
> +    // save safty values to list_src_images and list_size given through
param

Start the sentense with capital letter and add a "." in the end.

> Source/WebKit/efl/ewk/ewk_frame.cpp:2079
> +	   /*
> +	    * This section saves images only.

Use "//" comments instead of "/* */"

> Source/WebKit/efl/ewk/ewk_frame.cpp:2080
> +	    * TODO Support for others resources like scripts, plugins, external
css styles.

WebKit usually uses FIXME instead of TODO.

> Source/WebKit/efl/ewk/ewk_frame.cpp:2083
> +	   // getting collection of all images in the frame

Capital in the beginning, period in the end.

> Source/WebKit/efl/ewk/ewk_frame.cpp:2105
> +	       WebCore::HTMLImageElement *imageElement =
(WebCore::HTMLImageElement *) images->item(index);

* on the left side. WebKit uses static_cast for casting instead of the c-styled
(XXX *).

> Source/WebKit/efl/ewk/ewk_frame.cpp:2124
> +		       // set a src attribute of the image to the local path.

Capital and period in the end.

> Source/WebKit/efl/ewk/ewk_frame.cpp:2129
> +		   // create an another name for the file with a different src
attribute 

Ditto.

> Source/WebKit/efl/ewk/ewk_frame.cpp:2142
> +	       // check nulls

Remove this comment. It adds nothing.

> Source/WebKit/efl/ewk/ewk_frame.cpp:2170
> +	       // set a src attribute of image to the local path

Capital letter and period in the end.

> Source/WebKit/efl/ewk/ewk_frame.cpp:2174
> +	   } // end for(...)

Remove the comment.

> Source/WebKit/efl/ewk/ewk_frame.cpp:2176
> +	   // save html source of body

Capital letter and period in the end.

> Source/WebKit/efl/ewk/ewk_frame.cpp:2180
> +	   // restore original src attributes of images

Ditto.

> Source/WebKit/efl/ewk/ewk_frame.cpp:2182
> +	       WebCore::HTMLImageElement *imageElement =
(WebCore::HTMLImageElement *) images->item(index);

"*" on the left side, and static_cast.

> Source/WebKit/efl/ewk/ewk_frame.cpp:2188
> +    } else { // end if (save_resources)

Remove the comment.

> Source/WebKit/efl/ewk/ewk_frame.cpp:2209
> +    // save a head tag

Capital letter and period.

> Source/WebKit/efl/ewk/ewk_frame.cpp:2216
> +    // save a body tag

Ditto.

> Source/WebKit/efl/ewk/ewk_frame.cpp:2231
> +    // copy values of 'src_values' to the list given through param

Ditto