[Webkit-unassigned] [Bug 107301] methods on window.internals shouldn't pass in a document
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Oct 24 16:17:26 PDT 2013
https://bugs.webkit.org/show_bug.cgi?id=107301
Alexey Proskuryakov <ap at webkit.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #215110|commit-queue? |commit-queue-
Flag| |
--- Comment #19 from Alexey Proskuryakov <ap at webkit.org> 2013-10-24 16:16:11 PST ---
(From update of attachment 215110)
View in context: https://bugs.webkit.org/attachment.cgi?id=215110&action=review
This patch doesn't apply, so commit queue won't be able to land it.
> Source/WebCore/testing/Internals.cpp:1538
> +//The 'document' parameter was not removed because the document instance (got from
> +//contextDocument()) is not the same as the one passed in the Layout Tests
> +//(window.document, for example).
Some nitpicks:
1.There should be spaces after "//".
2. Saying why the argument was not removed is not the best comment that can be made. A person reading likely won't care about the history (and won't even know that we used to have the argument in other functions before). A comment should talk about current state, not about history.
I'd have said something like:
// FIXME: Remove the document argument. It is almost always the same as contextDocument(), with the exception of a few tests that pass a different document, and could just make the call through another Internals instance instead.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list