[Webkit-unassigned] [Bug 57014] New tests for CSS3: 2d-transforms

[bugzilla-daemon at webkit.org]

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


Dirk Schulze <krit at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #87336|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #9 from Dirk Schulze <krit at webkit.org>  2011-04-26 16:11:42 PST ---
(From update of attachment 87336)
View in context: https://bugs.webkit.org/attachment.cgi?id=87336&action=review

This needs a lot of clean up.

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:14
> +/*declared variables for expected values 
> + left difference, top difference, width difference, expected height difference */

Don't use multiline comments

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:18
> +var exp_left_diff=0; 
> +var exp_top_diff=0;
> +var exp_width_diff=0; 
> +var exp_height_diff=0; 

spaces before and after the =
No underscores. This is against WebKit style.

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:21
> +// Object initialV for initial values and object finalV for values after transformation

The indention of the following lines is wrong. Also, never use tabs.

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:23
> +     var initialV = new Object();
> +     var finalV = new Object();	

please use self explaining names. What is the V for? use initialValues. Why not just using intital and final and so on?

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:30
> +        // var result1 = document.getElementById ("result1");
> +	//     result1.innerHTML="<b>Before:</b> left:"+initialV.left+", top: "+initialV.top+", width:"+initialV.width+", height:" +initialV.height;  
> +       // calling applyTransform		 

Why is this commented out?

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:40
> +	//   var result2 = document.getElementById ("result2");
> +	 //   result2.innerHTML="<b>After:</b> left:"+finalV.left+", top: "+finalV.top+", width:"+finalV.width+", height:" +finalV.height; 

Ditto, Don't check commented out code in.

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:63
> +		/*2 pixels tolerance is given. We are validating difference in properties length. So 2 pixels positive side and 2 pixels in negative side are considered.  */ 		

Just use //

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:89
> +		if (left1==1&&top1==1&&width1==1&&height1==1)			

This is general code used by more than one test. Please use WebKit style here.

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:101
> +      var result=result1;

Ditto.

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:113
> +          resultElement.innerHTML="FAIL";
> +          resultElement.style.background="red";

Ditto

> LayoutTests/css3/css3-2d-transforms/js/logResults.js:3
> +if (window.layoutTestController)
> +       { layoutTestController.dumpAsText();

put the brace behind the condition. Indention of all lines is wrong. Don't use tabs

> LayoutTests/css3/css3-2d-transforms/js/logResults.js:8
> +function notify()
> +	{

aline brace and function. Better to put the brace behind the function

> LayoutTests/css3/css3-2d-transforms/js/logResults.js:13
> + function logResults(result1){

space missing. What does result1 mean? take better names please

> LayoutTests/css3/css3-2d-transforms/js/logResults.js:14
> +      var result=result1;

Why are you creating a new var herer?

> LayoutTests/css3/css3-2d-transforms/js/logResults.js:19
> +	      if (result=="PASS"){
> +          resultElement.innerHTML="PASS";
> +          resultElement.style.background="green";

Spaces please

> LayoutTests/css3/css3-2d-transforms/js/logResults.js:26
> +          resultElement.innerHTML="FAIL";
> +          resultElement.style.background="red";
> +		  notify();

Ditto.

> LayoutTests/css3/css3-2d-transforms/tests/2d_transform_animation_3-expected.txt:1
> +Prev [index] Next

Wrong text coding

> LayoutTests/css3/css3-2d-transforms/tests/2d_transform_animation_3.html:1
> +<!DOCTYPE html>

indention is wrong

> LayoutTests/css3/css3-2d-transforms/tests/2d_transform_cssMatrix_attributes.html:28
> +//var display= document.getElementById("display");
> +//display.innerHTML="a: "+m1.a +"<br>"+"b: "+m1.b +"<br>"+"c: "+m1.c +"<br>"+"d: "+m1.d +"<br>"+"e: "+m1.e +"<br>"+"f: "+m1.f;
> +//calling validate() function

no out commented lines. I don't continue here. Just remaining patterns.

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