Bug 76095 - HTMLIsIndexElement should not expose HTMLInputElement properties
Summary: HTMLIsIndexElement should not expose HTMLInputElement properties
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks: 77887
  Show dependency treegraph
 
Reported: 2012-01-11 12:32 PST by Eric Seidel (no email)
Modified: 2012-02-06 11:21 PST (History)
6 users (show)

See Also:


Attachments
I need tests still (1.20 KB, patch)
2012-01-11 12:33 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch (4.31 KB, patch)
2012-01-11 12:53 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch (57.44 KB, patch)
2012-01-11 14:18 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (4.23 KB, patch)
2012-01-25 14:54 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (47.87 KB, patch)
2012-01-25 18:54 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (47.86 KB, patch)
2012-01-25 18:58 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (47.86 KB, patch)
2012-01-25 19:03 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (57.59 KB, patch)
2012-01-26 03:48 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (68.97 KB, patch)
2012-01-26 19:31 PST, Eric Seidel (no email)
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (1.28 KB, patch)
2012-01-31 11:58 PST, Rafael Brandao
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2012-01-11 12:32:53 PST
HTMLIsIndexElement should not expose HTMLInputElement properties
Comment 1 Eric Seidel (no email) 2012-01-11 12:33:40 PST
Created attachment 122074 [details]
I need tests still
Comment 2 Eric Seidel (no email) 2012-01-11 12:34:54 PST
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.
Comment 3 Eric Seidel (no email) 2012-01-11 12:53:07 PST
Created attachment 122079 [details]
Patch
Comment 4 Eric Seidel (no email) 2012-01-11 12:55:23 PST
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. :)
Comment 5 Timothy Hatcher 2012-01-11 13:17:30 PST
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.
Comment 6 Eric Seidel (no email) 2012-01-11 14:18:07 PST
Created attachment 122096 [details]
Patch
Comment 7 Eric Seidel (no email) 2012-01-11 14:18:46 PST
I've updated the patch with 10x more deletion. :)
Comment 8 Adam Barth 2012-01-11 14:23:14 PST
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?
Comment 9 Eric Seidel (no email) 2012-01-11 14:24:06 PST
(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. :)
Comment 10 Eric Seidel (no email) 2012-01-25 14:54:31 PST
Created attachment 124018 [details]
Patch for landing
Comment 11 WebKit Review Bot 2012-01-25 16:18:16 PST
Comment on attachment 124018 [details]
Patch for landing

Clearing flags on attachment: 124018

Committed r105940: <http://trac.webkit.org/changeset/105940>
Comment 12 WebKit Review Bot 2012-01-25 16:18:21 PST
All reviewed patches have been landed.  Closing bug.
Comment 13 Mark Rowe (bdash) 2012-01-25 16:56:56 PST
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.
Comment 14 Adam Barth 2012-01-25 17:01:02 PST
(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.
Comment 15 Eric Seidel (no email) 2012-01-25 17:02:58 PST
My appologies.  I'm happy to fix the Xcode proj if you haven't already.
Comment 16 Eric Seidel (no email) 2012-01-25 17:09:16 PST
Reverted r105940 for reason:

Only half this change landed

Committed r105947: <http://trac.webkit.org/changeset/105947>
Comment 17 Eric Seidel (no email) 2012-01-25 17:11:59 PST
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.
Comment 18 Eric Seidel (no email) 2012-01-25 18:54:36 PST
Created attachment 124054 [details]
Patch for landing
Comment 19 WebKit Review Bot 2012-01-25 18:56:43 PST
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
Comment 20 Eric Seidel (no email) 2012-01-25 18:58:18 PST
Created attachment 124055 [details]
Patch for landing
Comment 21 WebKit Review Bot 2012-01-25 19:02:04 PST
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
Comment 22 Eric Seidel (no email) 2012-01-25 19:03:02 PST
Created attachment 124056 [details]
Patch for landing
Comment 23 WebKit Review Bot 2012-01-25 20:20:38 PST
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
Comment 24 Eric Seidel (no email) 2012-01-26 03:48:00 PST
Created attachment 124097 [details]
Patch for landing
Comment 25 WebKit Review Bot 2012-01-26 05:23:20 PST
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
Comment 26 Darin Adler 2012-01-26 10:53:28 PST
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.
Comment 27 Eric Seidel (no email) 2012-01-26 19:31:23 PST
Created attachment 124238 [details]
Patch for landing
Comment 28 WebKit Review Bot 2012-01-26 21:06:44 PST
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
Comment 29 Eric Seidel (no email) 2012-01-31 11:32:19 PST
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.
Comment 30 Eric Seidel (no email) 2012-01-31 11:48:00 PST
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.
Comment 31 Rafael Brandao 2012-01-31 11:53:18 PST
(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.
Comment 32 Eric Seidel (no email) 2012-01-31 11:58:30 PST
Oh boy.  The feeder queue must be down, if this is failing to go through the EWSes?
Comment 33 Rafael Brandao 2012-01-31 11:58:36 PST
Created attachment 124789 [details]
Patch
Comment 34 Eric Seidel (no email) 2012-01-31 11:59:33 PST
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
Comment 35 Eric Seidel (no email) 2012-01-31 11:59:47 PST
Comment on attachment 124789 [details]
Patch

Thanks.
Comment 36 Eric Seidel (no email) 2012-01-31 12:00:43 PST
Comment on attachment 124789 [details]
Patch

Clearing flags on attachment: 124789

Committed r106377: <http://trac.webkit.org/changeset/106377>
Comment 37 Eric Seidel (no email) 2012-01-31 12:00:51 PST
All reviewed patches have been landed.  Closing bug.
Comment 38 WebKit Review Bot 2012-01-31 12:00:54 PST
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.
Comment 39 Eric Seidel (no email) 2012-01-31 14:20:37 PST
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.
Comment 40 Gyuyoung Kim 2012-01-31 21:31:01 PST
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. :-)
Comment 41 Eric Seidel (no email) 2012-02-06 11:18:41 PST
(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.