Bug 39018

Summary:

[chromium] "Check spelling in this field" context menu item always checked

Product:

WebKit

Reporter:

Evan Stade <estade>

Component:

WebKit API

Assignee:

Evan Stade <estade>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, dglazkov, evan, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

All

Attachments:

Description	Flags
try1	jianli: review-
do initing in default constructor	none
syntax	none

Evan Stade

Reported 2010-05-12 12:58:23 PDT

http://code.google.com/p/chromium/issues/detail?id=42981 Note that the functionality is not broken, just the state of the check. This affects all chromium platforms.

Attachments
try1 (1.48 KB, patch) 2010-05-12 14:19 PDT, Evan Stade	jianli: review-	Details Formatted Diff Diff
do initing in default constructor (3.44 KB, patch) 2010-05-13 12:40 PDT, Evan Stade	no flags	Details Formatted Diff Diff
syntax (3.44 KB, patch) 2010-05-13 13:02 PDT, Evan Stade	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Evan Stade

Comment 1 2010-05-12 14:19:37 PDT

Created attachment 55899 [details] try1

Evan Martin

Comment 2 2010-05-13 00:13:15 PDT

Is "data" here some struct? Maybe it'd make more sense to just memset it to zero when it's created?

Evan Stade

Comment 3 2010-05-13 10:49:18 PDT

yes, data is a struct. Memsetting doesn't seem like it would produce the most intuitive code.

Jian Li

Comment 4 2010-05-13 12:17:42 PDT

Comment on attachment 55899 [details] try1 I think we should either introduce default constructor or memset the struct instance in order to avoid future mistake when we add new members.

Evan Stade

Comment 5 2010-05-13 12:40:50 PDT

Created attachment 56007 [details] do initing in default constructor

WebKit Review Bot

Comment 6 2010-05-13 12:59:47 PDT

Attachment 56007 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/2253037

Evan Stade

Comment 7 2010-05-13 13:02:06 PDT

Created attachment 56009 [details] syntax

Adam Barth

Comment 8 2010-05-13 13:30:07 PDT

Comment on attachment 56009 [details] syntax We usually put constructors at the top of the declaration. It's a bit odd to have a struct with a constructor, but I think we do that in other places too.

Evan Stade

Comment 9 2010-05-13 14:23:42 PDT

all the structs I checked in WebKit/chromium/public/ (WebRect, WebSize, WebFindOptions, WebScriptSource, etc.) put the constructor(s) after the data members.

Adam Barth

Comment 10 2010-05-14 11:33:43 PDT

Comment on attachment 56009 [details] syntax Ok. We might want to change all of those in the future. Thanks for being consistent with nearby existing code.

WebKit Commit Bot

Comment 11 2010-05-15 02:21:56 PDT

Comment on attachment 56009 [details] syntax Clearing flags on attachment: 56009 Committed r59529: <http://trac.webkit.org/changeset/59529>

WebKit Commit Bot

Comment 12 2010-05-15 02:22:02 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.