Brent Fulgham

Results 16 comments of Brent Fulgham

Yes, this led to some confusion on my part when debugging something.

It would be great to see this patch get landed. Is there anything we can do to help make progress?

@darinadler I think I've addressed all of your review recommendations, and found the cause of the test failure. You moved from Approved to "Request Changes" in your last review. If...

The Windows failure was an out-of-space error on the bot. The API failures don't reproduce locally (though I get other crashes). I am opening a separate bug to investigate some...

Note that two of the test cases in this change cause crashes on Big Sur machines (and likely older). See https://github.com/WebKit/WebKit/pull/4836 for further discussion.

I do not believe the three GTK API test failures are related to this change. They involve Accessibility features, and are not related to the navigator.plugin feature or the results...

Note: This is a WIP. New tests are not in this patch yet.

> > This patch implements > > this part of the spec, and adds tests. > > Where are the tests? They should be in the PR now.

I'm skeptical of the API test failure, since the tests have been passing consistently in earlier patches, and I've only been adjusting DumpRenderTree and test cases, not engine code in...

> Not done reviewing but already setting r- to make sure this doesn't land given this PR got approved and has significant bugs. I spoke with Per Arne offline, and...