[Webkit-unassigned] [Bug 207748] Null Ptr Deref @ WebCore::Node::Treescope

Fri Feb 14 00:50:34 PST 2020

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

Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #390735|review?, commit-queue?      |review-, commit-queue-
              Flags|                            |

--- Comment #2 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 390735
  --> https://bugs.webkit.org/attachment.cgi?id=390735
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=390735&action=review

r- due to the issues listed below.

> Source/WebCore/ChangeLog:6
> +        Reviewed by Ryosuke Niwa. 

This is definitely not reviewed by me yet. Please undo this and say NOBODY (OOPS!).

> Source/WebCore/html/HTMLTextFormControlElement.cpp:307
> +        if (!isTextField())
> +            return;

Let's check this before getting innerText. It's weird to try to update innerText if it's not a text field

> LayoutTests/ChangeLog:6
> +        Reviewed by Ryosuke Niwa NOBODY.

Ditto. This is definitely not reviewed by me.

In fact, since I wrote this case, it's not appropriate for me to review this test.

> LayoutTests/ChangeLog:8
> +        Added a regression test to verify the fix

Missing a period at the end. Also, we need to give the test author (me in this case) a credit in this case.
Say something like: "Added a regression test to verify the fix based on a test case written by Ryosuke Niwa".

> LayoutTests/editing/selection/ignore-selection-range-on-input-style-change.html:1
> +<html>

Missing <!DOCTYPE html>.

> LayoutTests/editing/selection/ignore-selection-range-on-input-style-change.html:11
> +    const input3 = document.createElement('input');

Please rename this to inputWithAutofocus or something.

> LayoutTests/editing/selection/ignore-selection-range-on-input-style-change.html:22
> +

Please remove this blank lines.

> LayoutTests/editing/selection/ignore-selection-range-on-input-style-change.html:24
> +<p>Testcase passes if there is no crash </p>

"Testcase" should be "test case" but I think we should just say "This test".

> LayoutTests/editing/selection/ignore-selection-range-on-input-style-change.html:25
> +

Ditto.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200214/a5e3349c/attachment.htm>