Bug 39018 - [chromium] "Check spelling in this field" context menu item always checked
Summary: [chromium] "Check spelling in this field" context menu item always checked
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Evan Stade
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-12 12:58 PDT by Evan Stade
Modified: 2010-05-15 02:22 PDT (History)
4 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Evan Stade 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.
Comment 1 Evan Stade 2010-05-12 14:19:37 PDT
Created attachment 55899 [details]
try1
Comment 2 Evan Martin 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?
Comment 3 Evan Stade 2010-05-13 10:49:18 PDT
yes, data is a struct. Memsetting doesn't seem like it would produce the most intuitive code.
Comment 4 Jian Li 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.
Comment 5 Evan Stade 2010-05-13 12:40:50 PDT
Created attachment 56007 [details]
do initing in default constructor
Comment 6 WebKit Review Bot 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
Comment 7 Evan Stade 2010-05-13 13:02:06 PDT
Created attachment 56009 [details]
syntax
Comment 8 Adam Barth 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.
Comment 9 Evan Stade 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.
Comment 10 Adam Barth 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.
Comment 11 WebKit Commit Bot 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>
Comment 12 WebKit Commit Bot 2010-05-15 02:22:02 PDT
All reviewed patches have been landed.  Closing bug.