[Webkit-unassigned] [Bug 68684] spec change - option.label should be reflected like option.value

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

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




--- Comment #13 from Darin Adler <darin at apple.com>  2011-09-30 09:50:39 PST ---
(From update of attachment 109285)
View in context: https://bugs.webkit.org/attachment.cgi?id=109285&action=review

commit-queue- because the comments still need a little work and the copyrights are a bit wrong too

> Source/WebCore/dom/OptionElement.cpp:3
> + * Portions Copyright (c) 2010 Motorola Mobility, Inc.  All rights reserved.

Should not add copyright since you are just re-ordering includes and removing a space in a comment.

> Source/WebCore/dom/OptionElement.h:3
> + * Portions Copyright (c) 2010 Motorola Mobility, Inc.  All rights reserved.

Doesn’t make sense to add copyright just for moving private down a line.

> Source/WebCore/html/HTMLOptionElement.cpp:8
> + * Portions Copyright (c) 2010 Motorola Mobility, Inc.  All rights reserved.

Would be better if your copyright matched the format of the others. Also, 2011 is the year of publication here, not 2010. Copyright should cite year of publication.

> Source/WebCore/html/HTMLOptionElement.cpp:225
> +    // Use the text if the Label wasn't set. Also strip leading and trailing html whitespace.

This comment says what we are doing, but not why. Comments need to say why we do things, not just repeat what the code does and say what we are doing.

Also, "label" should not be capitalized, "HTML" should be capitalized.

> Source/WebCore/html/HTMLOptionElement.cpp:228
> +    // we want to collapse our html whitespace within the text too.

This comment says what we are doing, but not why. Comments need to say why we do things, not just repeat what the code does and say what we are doing.

Also, "We" should be capitalized and "HTML" should be capitalized.

> Source/WebCore/html/HTMLOptionElement.h:7
> + * Portions Copyright (c) 2010 Motorola Mobility, Inc.  All rights reserved.

The year is 2011. Copyright dates are based on year of publication. Also I’m not sure that just adding a single-line function declaration justifies adding a copyright line here.

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