Bug 36698 – There are build breaks when wml is enabled.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Gyuyoung Kim]

Created attachment 51823 [details]
Patch for a build break due to the disabled() funciton

There is a build break due to the disabled() when wml is enabled. It seems that this break comes from the Bug 35056 - Disabled menu options are still submitted. 

WMLOptionElement class is a child class of OptionElement class as below. So, if a virtual function is added to OptionElement, WMLOptionElement class should defines that function as well.

  class WMLOptionElement : public WMLFormControlElement, public WMLEventHandlingElement, public OptionElement {



My modification is as below,

--- WebCore/wml/WMLOptionElement.h      (revision 56658)
+++ WebCore/wml/WMLOptionElement.h      (working copy)
@@ -41,6 +41,8 @@ public:
     virtual void childrenChanged(bool changedByParser = false, Node* beforeChange = 0, Node* afterChange = 0, int childCountDelta = 0);
     virtual void parseMappedAttribute(MappedAttribute*);

+    virtual bool disabled() const { return false; };
+

I think diabled() should be added to WMLOptionElement.h.

Thank you,
Gyuyoung Kim.

[Gyuyoung Kim]

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.

[Darin Adler]

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

[WebKit Commit Bot]

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

[Gyuyoung Kim]

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.

[WebKit Review Bot]

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.

[Eric Seidel (no email)]

Comment on attachment 51993 [details]
 Patch for a build break due to the disabled() function 

Actually, no need for hte extra ";" after the }

[Gyuyoung Kim]

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.

[Eric Seidel (no email)]

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.

[Gyuyoung Kim]

Thanks. When I push another patches, I will write changelog correctly.

[WebKit Commit Bot]

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

[Gyuyoung Kim]

My patch is wrong ?  Should I make a patch again ?

[Eric Seidel (no email)]

Your patch looks fine.  Looks like Adam or I broke the commit-queue.  I'll investigate.

[Eric Seidel (no email)]

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.

[Eric Seidel (no email)]

There was also a problem in webkit-patch's validatereviewer.py step which I've fixed.

[Gyuyoung Kim]

Created attachment 52007 [details]
Patch for a build break due to the disabled() function

Thank you for your guide. I attach a patch again.

[Eric Seidel (no email)]

Comment on attachment 52007 [details]
Patch for a build break due to the disabled() function

Thanks

[WebKit Commit Bot]

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>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.