[Webkit-unassigned] [Bug 38188] Remove custom NodeIterator bindings

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=38188


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |darin at apple.com




--- Comment #10 from Darin Adler <darin at apple.com>  2010-04-27 12:01:43 PST ---
(In reply to comment #6)
> > WebCore/bindings/scripts/test/JS/JSTestObj.cpp:640
> >  +      setDOMException(exec, ec);
> > This will smash over an an existing exception on the exec it seems.  Seems
> > wrong.
> 
> As discussed before, that's not how setDOMException works.

Typically in a case like this we construct a test case demonstrating that's OK.

> > WebCore/bindings/scripts/test/JS/JSTestObj.cpp:639
> >  +      JSC::JSValue result = toJS(exec, castedThisObj->globalObject(),
> > WTF::getPtr(imp->withScriptStateObjException(exec, ec)));
> > This will probably cause this to actually fix isolated world bugs.  You may
> > need to update test results.
> 
> We're going to fix lots of random bugs in this proces.  This didn't cause any
> tests to fail AFAIK.

Our project policy is normally that we create test cases when we notice bugs
and fix them. Even if the reason for the code change in the first place was not
that particular bug. I don’t think “we’re going to fix lots of random bugs” is
a convincing argument.

Whether an existing test covers this or not is not the point.

This won’t prevent me from approving this patch, but I think we should continue
to create test cases for bugs we fix. I spend quite a bit of time creating test
cases for bugs I notice while fixing another bug, and this seems like the same
sort of thing.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.