Bug 76095 – HTMLIsIndexElement should not expose HTMLInputElement properties

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

[Eric Seidel (no email)]

HTMLIsIndexElement should not expose HTMLInputElement properties

[Eric Seidel (no email)]

Created attachment 122074 [details]
I need tests still

[Eric Seidel (no email)]

Since this will be an API change for Obj-C, we'll want Tim to see this.

This came up due to IETC: http://samples.msdn.microsoft.com/ietestcenter/dominheritance/showdominheritancetest.htm?Prototype_HTMLIsIndexElement

DOM 2 is pretty clear on this:
http://www.w3.org/TR/DOM-Level-2-HTML/html.html#ID-85283003

Since this is a deprecated element anyway, it makes sense to expose less on it.

[Eric Seidel (no email)]

Created attachment 122079 [details]
Patch

[Eric Seidel (no email)]

Hmm.  It's actually technically supposed to be an HTMLUnknownElement in HTML5:
http://www.whatwg.org/specs/web-apps/current-work/#dfnReturnLink-6

Although this patch is OK, we could go even further. :)

[Timothy Hatcher]

I dont see ObjC changes in the patch. The PublicDOMInterfaces.h should be touched. This element is not important, so changing the ObjC likely wont have an impact.

[Eric Seidel (no email)]

Created attachment 122096 [details]
Patch

[Eric Seidel (no email)]

I've updated the patch with 10x more deletion. :)

[Adam Barth]

Comment on attachment 122096 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=122096&action=review

> Source/WebCore/bindings/objc/PublicDOMInterfaces.h:-633
> -@interface DOMHTMLIsIndexElement : DOMHTMLInputElement WEBKIT_VERSION_1_3
> -@property(readonly, retain) DOMHTMLFormElement *form;
> -@property(copy) NSString *prompt;
> -@end

This isn't going to cause problems?

[Eric Seidel (no email)]

(In reply to comment #8)
> (From update of attachment 122096 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=122096&action=review
> 
> > Source/WebCore/bindings/objc/PublicDOMInterfaces.h:-633
> > -@interface DOMHTMLIsIndexElement : DOMHTMLInputElement WEBKIT_VERSION_1_3
> > -@property(readonly, retain) DOMHTMLFormElement *form;
> > -@property(copy) NSString *prompt;
> > -@end
> 
> This isn't going to cause problems?

That's up to Tim to decide.  I'll let him comment before landing.  I really doubt it.  This element has been deprecated for some 10+ years. :)

[Eric Seidel (no email)]

Created attachment 124018 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 124018 [details]
Patch for landing

Clearing flags on attachment: 124018

Committed r105940: <http://trac.webkit.org/changeset/105940>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Mark Rowe (bdash)]

r105940 broke the build as it no longer generates DOMHTMLIsIndexElement.h but it's still present in the Xcode project and in WebKit/mac's MigrateHeaders.make.

[Adam Barth]

(In reply to comment #13)
> r105940 broke the build as it no longer generates DOMHTMLIsIndexElement.h but it's still present in the Xcode project and in WebKit/mac's MigrateHeaders.make.

I pinged Eric.  He says he'll fixenate.

[Eric Seidel (no email)]

My appologies.  I'm happy to fix the Xcode proj if you haven't already.

[Eric Seidel (no email)]

Reverted r105940 for reason:

Only half this change landed

Committed r105947: <http://trac.webkit.org/changeset/105947>

[Eric Seidel (no email)]

This was yet another example of bug 77057 biting us. :)  Thankfully that's now fixed.  Hopefully we won't have this kind of half-apply confusion in the future.

[Eric Seidel (no email)]

Created attachment 124054 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 124054 [details]
Patch for landing

Rejecting attachment 124054 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/11163153

[Eric Seidel (no email)]

Created attachment 124055 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 124055 [details]
Patch for landing

Rejecting attachment 124055 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/Source/WebKit/mac/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/11191160

[Eric Seidel (no email)]

Created attachment 124056 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 124056 [details]
Patch for landing

Rejecting attachment 124056 [details] from commit-queue.

New failing tests:
fast/dom/element-attribute-js-null.html
fast/dom/dom-constructors.html
fast/dom/Window/window-lookup-precedence.html
fast/dom/wrapper-classes.html
fast/forms/implicit-submission.html
fast/dom/Window/element-constructors-on-window.html
Full output: http://queues.webkit.org/results/11163172

[Eric Seidel (no email)]

Created attachment 124097 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 124097 [details]
Patch for landing

Rejecting attachment 124097 [details] from commit-queue.

New failing tests:
fast/dom/Window/window-lookup-precedence.html
fast/dom/wrapper-classes.html
fast/forms/implicit-submission.html
fast/dom/Window/element-constructors-on-window.html
Full output: http://queues.webkit.org/results/11297633

[Darin Adler]

Comment on attachment 124097 [details]
Patch for landing

After landing a patch like this there is also some cleanup that can be done for HTMLInputElement since it no longer has any classes derived from it. There are functions and features of HTMLInputElement that are only used by HTMLIsIndexElement.

[Eric Seidel (no email)]

Created attachment 124238 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 124238 [details]
Patch for landing

Rejecting attachment 124238 [details] from commit-queue.

New failing tests:
fast/dom/Window/window-lookup-precedence.html
fast/dom/wrapper-classes.html
fast/forms/implicit-submission.html
fast/dom/Window/element-constructors-on-window.html
Full output: http://queues.webkit.org/results/11314889

[Eric Seidel (no email)]

These tests pass locally on Mac.  This is fallout from v8/jsc differences and our insane cascade of expectations.  I believe the best path forward is to land this and then rebaseline things from the bots.  I'll notify the chromium gardner.

[Eric Seidel (no email)]

This appears to have landed in http://trac.webkit.org/changeset/106373 but not updated the bug.  I'll take a peak at the bots in a bit and see how much rebaselining is needed.

[Rafael Brandao]

(In reply to comment #30)
> This appears to have landed in http://trac.webkit.org/changeset/106373 but not updated the bug.  I'll take a peak at the bots in a bit and see how much rebaselining is needed.

This caused a build issue on qt, I'll upload a small patch here to fix it.

[Eric Seidel (no email)]

Oh boy.  The feeder queue must be down, if this is failing to go through the EWSes?

[Rafael Brandao]

Created attachment 124789 [details]
Patch

[Eric Seidel (no email)]

Oh, nm, these were all uploaded as "patch for landing" thus were never fed.  The feeder-queue is fine: http://queues.webkit.org/active-bots

[Eric Seidel (no email)]

Comment on attachment 124789 [details]
Patch

Thanks.

[Eric Seidel (no email)]

Comment on attachment 124789 [details]
Patch

Clearing flags on attachment: 124789

Committed r106377: <http://trac.webkit.org/changeset/106377>

[Eric Seidel (no email)]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Attachment 124789 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
From git://git.webkit.org/WebKit
   c00f0a0..69e2a16  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 106373 = c00f0a0c31f2721d2fb62ca757c315e016c7c7b5
r106375 = 69e2a16f0d5685b4abb632e3fad459685b7487f7
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
	M	LayoutTests/platform/chromium/test_expectations.txt
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/platform/graphics/gpu/DrawingBuffer.cpp
r106376 = b20e8a895f05dd841cd264088d3b83af0e98a788 (refs/remotes/origin/master)
First, rewinding head to replay your work on top of it...
Applying: Fix compilation errors on build-webkit --debug --no-workers on mac.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/qt/Skipped
CONFLICT (content): Merge conflict in LayoutTests/platform/qt/Skipped
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Auto-merging Source/WebCore/WebCore.exp.in
Auto-merging Source/WebKit/mac/ChangeLog
CONFLICT (content): Merge conflict in Source/WebKit/mac/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 Fix compilation errors on build-webkit --debug --no-workers on mac.

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Seidel (no email)]

I've rebaselined chromium using the awesome garden-o-matic.  I believe there may be other failures due to custom results for various other ports, but no good tool to do those rebaselines (or even see the failures).  I'm not convinced that http://build.webkit.org/TestFailures is actually workign right now.

[Gyuyoung Kim]

Hello Eric,

There was build break in EFL port because of this patch. I fix it on r106427.
http://trac.webkit.org/changeset/106427

Please consider EFL port as well in future. :-)

[Eric Seidel (no email)]

(In reply to comment #26)
> (From update of attachment 124097 [details])
> After landing a patch like this there is also some cleanup that can be done for HTMLInputElement since it no longer has any classes derived from it. There are functions and features of HTMLInputElement that are only used by HTMLIsIndexElement.

MediaControlInputElement and UploadButtonElement appear to derive from it.  But I'm looking at removing other cruft from isindex now.