Bug 82795 - Add tests for iframe seamless and support for parsing webkitseamless attribute
: Add tests for iframe seamless and support for parsing webkitseamless attribute
Status: RESOLVED LATER
Product: WebKit
Classification: Unclassified
Component: New Bugs
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: Eric Seidel
:
Depends on:
Blocks: 45950
  Show dependency treegraph
 
Reported: 2012-03-30 15:50 PDT by Eric Seidel
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 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 2012-03-30 15:50:24 PDT
Add tests for iframe seamless and support for parsing webkitseamless attribute
Comment 1 Eric Seidel 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 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 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 2012-04-01 17:46:31 PDT
The GitHub branch is here:
https://github.com/eseidel/webkit/compare/master...seamless