Summary: | There are build breaks when wml is enabled. | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gyuyoung Kim <gyuyoung.kim> | ||||||||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | abarth, commit-queue, eric, joone.hur, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | PC | ||||||||||||||
OS: | Linux | ||||||||||||||
Attachments: |
|
Description
Gyuyoung Kim
2010-03-27 02:55:58 PDT
Created attachment 51872 [details]
Patch for a build break due to the disabled() function
I add a bug url to patch. Please review my patch.
Comment on attachment 51872 [details]
Patch for a build break due to the disabled() function
I'm not sure that's right. Is it true that WML option elements can't be disabled? We can follow up with another patch if the WML behavior is wrong, I guess. r=me
Comment on attachment 51872 [details] Patch for a build break due to the disabled() function Rejecting patch 51872 from commit-queue. Failed to run "['/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', '--reviewer', 'Darin Adler', '--force']" exit_code: 2 patching file WebCore/ChangeLog patch: **** malformed patch at line 20: patching file WebCore/wml/WMLOptionElement.h Full output: http://webkit-commit-queue.appspot.com/results/1626054 Created attachment 51993 [details]
Patch for a build break due to the disabled() function
When wml feature is enabled, there is a build break as below,
------------------------------------------------------------------------------
DerivedSources/WMLElementFactory.cpp: In function
‘WTF::PassRefPtr<WebCore::WMLElement> WebCore::optionConstructor(const
WebCore::QualifiedName&, WebCore::Document*, bool)’:
DerivedSources/WMLElementFactory.cpp:154: error: cannot allocate an object of
abstract type ‘WebCore::WMLOptionElement’
./WebCore/wml/WMLOptionElement.h:31: note: because the following virtual
functions are pure within ‘WebCore::WMLOptionElement’:
./WebCore/dom/OptionElement.h:37: note: virtual bool
WebCore::OptionElement::disabled() const
DerivedSources/WMLElementFactory.cpp: In function
‘WTF::PassRefPtr<WebCore::WMLElement> WebCore::selectConstructor(const
WebCore::QualifiedName&, WebCore::Document*, bool)’:
DerivedSources/WMLElementFactory.cpp:179: error: cannot allocate an object of
abstract type ‘WebCore::WMLSelectElement’
./WebCore/wml/WMLSelectElement.h:30: note: because the following virtual
functions are pure within ‘WebCore::WMLSelectElement’:
./WebCore/dom/SelectElement.h:64: note: virtual void
WebCore::SelectElement::listBoxSelectItem(int, bool, bool, bool)
------------------------------------------------------------------------------
I didn't check if the WML behavior is wrong. But, I think we need to avoid a build break for now.
I will report if WML option elements can't be disabled or not again. If I am wrong, please let me know.
Thank you.
Attachment 51993 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/wml/WMLOptionElement.h:44: More than one command on the same line [whitespace/newline] [4]
Total errors found: 1 in 2 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 51993 [details]
Patch for a build break due to the disabled() function
Actually, no need for hte extra ";" after the }
Created attachment 52005 [details] Patch for a build break due to the disabled() function Sorry for my frequent patches, I am not good at making a patch yet. I found one more build break because of Bug 36177 - Multiselect Popup - Listbox click simulation. (https://bugs.webkit.org/show_bug.cgi?id=36177) The patch add a virtual function to SelectElement.h as below, virtual void listBoxSelectItem(int listIndex, bool allowMultiplySelections, bool shift, bool fireOnChangeNow = true) = 0; It seems the patch of bug didn't consider wml. I add a virtual function to WMLSelectElement.h. Comment on attachment 52005 [details]
Patch for a build break due to the disabled() function
OK. Your ChangeLog does not mention WebCore/wml/WMLSelectElement.h, but it's OK.
Thanks. When I push another patches, I will write changelog correctly. Comment on attachment 52005 [details]
Patch for a build break due to the disabled() function
Rejecting patch 52005 from commit-queue.
Unexpected failure when landing patch! Please file a bug against webkit-patch.
Failed to run "['WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', '--build-style=both', '--quiet', '52005', '--no-update']" exit_code: 1
Last 500 characters of output:
handle_errors
self._run(tool, options, state)
File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/tool/commands/stepsequence.py", line 60, in _run
step(tool, options).run(state)
File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 63, in run
log("%s found in %s does not appear to be a valid reviewer according to committers.py." % (reviewer_text, changelog_path))
NameError: global name 'log' is not defined
My patch is wrong ? Should I make a patch again ? Your patch looks fine. Looks like Adam or I broke the commit-queue. I'll investigate. Comment on attachment 52005 [details]
Patch for a build break due to the disabled() function
Actually, looks like you modified the "reviewed by" line:
+ Reviewed by NOBODY(OOPS!).
the default template is:
+ Reviewed by NOBODY (OOPS!).
The scripts aren't smart enough to know the difference between "NOBODY(OOPS!)" and an intentionally specified reviewer.
I suggest just regenerating the ChangeLog from prepare-ChangeLog, which will both fix your reviewed by line to be the default as well as add the information about WebCore/wml/WMLSelectElement.h
Sorry our scripts are so troublesome.
There was also a problem in webkit-patch's validatereviewer.py step which I've fixed. Created attachment 52007 [details]
Patch for a build break due to the disabled() function
Thank you for your guide. I attach a patch again.
Comment on attachment 52007 [details]
Patch for a build break due to the disabled() function
Thanks
Comment on attachment 52007 [details] Patch for a build break due to the disabled() function Clearing flags on attachment: 52007 Committed r56769: <http://trac.webkit.org/changeset/56769> All reviewed patches have been landed. Closing bug. |