Bug 82795

Summary: Add tests for iframe seamless and support for parsing webkitseamless attribute
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED LATER    
Severity: Normal CC: abarth, ap, ojan, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 45950    
Attachments:
Description Flags
Patch none

Eric Seidel (no email)
Reported 2012-03-30 15:50:24 PDT
Add tests for iframe seamless and support for parsing webkitseamless attribute
Attachments
Patch (13.29 KB, patch)
2012-03-30 15:52 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2012-03-30 15:52:10 PDT
Created attachment 134904 [details] Patch
Adam Barth
Comment 2 2012-03-30 15:57:04 PDT
Comment on attachment 134904 [details] Patch Do we need to announce that we're implementing seamless to webkit-dev ?
Eric Seidel (no email)
Comment 3 2012-03-30 16:02:46 PDT
(In reply to comment #2) > (From update of attachment 134904 [details]) > Do we need to announce that we're implementing seamless to webkit-dev ? Done: https://lists.webkit.org/pipermail/webkit-dev/2012-March/020158.html
Ojan Vafai
Comment 4 2012-03-30 16:05:57 PDT
YAY!
WebKit Review Bot
Comment 5 2012-03-30 18:37:22 PDT
Comment on attachment 134904 [details] Patch Clearing flags on attachment: 134904 Committed r112760: <http://trac.webkit.org/changeset/112760>
WebKit Review Bot
Comment 6 2012-03-30 18:37:26 PDT
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 7 2012-03-31 10:42:34 PDT
I've seen it multiple times now that people first land failing tests, and then start implementing a feature. Where is this idea coming from? I think that this is exceptionally unhelpful and wrong. The reviewer looking at actual patch won't see what test coverage we have. Having ad hoc tests for unimplemented features is just a weird state for the code base to be in. > // FIXME: webkit prefix will be removed when implementation is complete. So why is this not behind an ifdef? A port that branches for release now won't want to expose this implementation artifact to the web.
Adam Barth
Comment 8 2012-03-31 11:12:00 PDT
> I've seen it multiple times now that people first land failing tests, and then start implementing a feature. Where is this idea coming from? I can't speak for Eric, but I often write patches this way. It's advocated by some folks under the name test-driven development: http://en.wikipedia.org/wiki/Test-driven_development > > // FIXME: webkit prefix will be removed when implementation is complete. > > So why is this not behind an ifdef? A port that branches for release now won't want to expose this implementation artifact to the web. There's a discussion about this point on webkit-dev. If you're interested in this topic, I encourage you to share your thought there.
Alexey Proskuryakov
Comment 9 2012-03-31 11:22:56 PDT
> It's advocated by some folks under the name test-driven development: Writing tests first is what I strongly advocate, too. Landing these is an entirely different matter.
Eric Seidel (no email)
Comment 10 2012-04-01 16:12:34 PDT
Reverted r112760 for reason: Revert addition of webkitseamless. I'll do this work on GitHub instead to avoid any half-implemented feature concerns. Committed r112820: <http://trac.webkit.org/changeset/112820>
Eric Seidel (no email)
Comment 11 2012-04-01 17:46:31 PDT
The GitHub branch is here: https://github.com/eseidel/webkit/compare/master...seamless
Note You need to log in before you can comment on or make changes to this bug.