Bug 82795 - Add tests for iframe seamless and support for parsing webkitseamless attribute
Summary: Add tests for iframe seamless and support for parsing webkitseamless attribute
Status: RESOLVED LATER
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: 45950
  Show dependency tree / graph
 
Reported: 2012-03-30 15:50 PDT by Eric Seidel (no email)
Modified: 2012-04-01 17:46 PDT (History)
4 users (show)

See Also:

Attachments
Patch (13.29 KB, patch)
2012-03-30 15:52 PDT, Eric Seidel (no email) 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2012-03-30 15:50:24 PDT 
Add tests for iframe seamless and support for parsing webkitseamless attribute
Comment 1 Eric Seidel (no email) 2012-03-30 15:52:10 PDT 
Created attachment 134904 [details]
Patch
Comment 2 Adam Barth 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 ?
Comment 3 Eric Seidel (no email) 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
Comment 4 Ojan Vafai 2012-03-30 16:05:57 PDT 
YAY!
Comment 5 WebKit Review Bot 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>
Comment 6 WebKit Review Bot 2012-03-30 18:37:26 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 7 Alexey Proskuryakov 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.
Comment 8 Adam Barth 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.
Comment 9 Alexey Proskuryakov 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.
Comment 10 Eric Seidel (no email) 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>
Comment 11 Eric Seidel (no email) 2012-04-01 17:46:31 PDT 
The GitHub branch is here:
https://github.com/eseidel/webkit/compare/master...seamless