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
: WebKit
New Bugs
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
: 45950
  Show dependency treegraph
 
Reported: 2012-03-30 15:50 PST by
Modified: 2012-04-01 17:46 PST (History)


Attachments
Patch (13.29 KB, patch)
2012-03-30 15:52 PST, Eric Seidel
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-03-30 15:50:24 PST
Add tests for iframe seamless and support for parsing webkitseamless attribute
------- Comment #1 From 2012-03-30 15:52:10 PST -------
Created an attachment (id=134904) [details]
Patch
------- Comment #2 From 2012-03-30 15:57:04 PST -------
(From update of attachment 134904 [details])
Do we need to announce that we're implementing seamless to webkit-dev ?
------- Comment #3 From 2012-03-30 16:02:46 PST -------
(In reply to comment #2)
> (From update of attachment 134904 [details] [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 From 2012-03-30 16:05:57 PST -------
YAY!
------- Comment #5 From 2012-03-30 18:37:22 PST -------
(From update of attachment 134904 [details])
Clearing flags on attachment: 134904

Committed r112760: <http://trac.webkit.org/changeset/112760>
------- Comment #6 From 2012-03-30 18:37:26 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #7 From 2012-03-31 10:42:34 PST -------
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 From 2012-03-31 11:12:00 PST -------
> 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 From 2012-03-31 11:22:56 PST -------
> 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 From 2012-04-01 16:12:34 PST -------
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 From 2012-04-01 17:46:31 PST -------
The GitHub branch is here:
https://github.com/eseidel/webkit/compare/master...seamless