Pranav Modi (Gerrit)

unread,

Mar 4, 2025, 7:43:44 AM3/4/25

to Sambamurthy Bandaru, Dan Clark, Sanket Joshi, Utkarsh Pathak, Rakesh Goulikar, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Dan Clark, Sambamurthy Bandaru, Sanket Joshi and Utkarsh Pathak

Pranav Modi added 1 comment

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Pranav Modi . resolved

Please review!

Open in Gerrit

Related details

Attention is currently required from:

Dan Clark
Sambamurthy Bandaru
Sanket Joshi
Utkarsh Pathak

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Pranav Modi (Gerrit)

unread,

Mar 5, 2025, 12:18:21 PM3/5/25

to Tanu Jain, Sambamurthy Bandaru, Dan Clark, Sanket Joshi, Utkarsh Pathak, Rakesh Goulikar, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Dan Clark, Sambamurthy Bandaru, Sanket Joshi, Tanu Jain and Utkarsh Pathak

Pranav Modi added 1 comment

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Pranav Modi . resolved

Requesting review from everyone!

Open in Gerrit

Related details

Attention is currently required from:

Dan Clark
Sambamurthy Bandaru
Sanket Joshi

Tanu Jain
Utkarsh Pathak

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Utkarsh Pathak (Gerrit)

unread,

Mar 5, 2025, 1:16:51 PM3/5/25

to Pranav Modi, Tanu Jain, Sambamurthy Bandaru, Dan Clark, Sanket Joshi, Rakesh Goulikar, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Dan Clark, Pranav Modi, Sambamurthy Bandaru, Sanket Joshi and Tanu Jain

Utkarsh Pathak voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Dan Clark
Pranav Modi

Sambamurthy Bandaru
Sanket Joshi
Tanu Jain

satisfied_requirement

unsatisfied_requirement

open

diffy

Pranav Modi (Gerrit)

unread,

Mar 5, 2025, 1:18:48 PM3/5/25

to Kent Tamura, Mason Freed, Utkarsh Pathak, Tanu Jain, Sambamurthy Bandaru, Dan Clark, Sanket Joshi, Rakesh Goulikar, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Dan Clark, Kent Tamura, Mason Freed, Sambamurthy Bandaru, Sanket Joshi and Tanu Jain

Pranav Modi added 1 comment

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Pranav Modi . resolved

Please review!

Open in Gerrit

Related details

Attention is currently required from:

Dan Clark
Kent Tamura
Mason Freed

Sambamurthy Bandaru
Sanket Joshi
Tanu Jain

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Kent Tamura (Gerrit)

unread,

Mar 5, 2025, 5:21:33 PM3/5/25

to Pranav Modi, Kent Tamura, Mason Freed, Utkarsh Pathak, Tanu Jain, Sambamurthy Bandaru, Dan Clark, Sanket Joshi, Rakesh Goulikar, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Dan Clark, Mason Freed, Pranav Modi, Sambamurthy Bandaru, Sanket Joshi and Tanu Jain

Kent Tamura voted and added 1 comment

Votes added by Kent Tamura

Code-Review

+1

1 comment

File third_party/blink/web_tests/external/wpt/editing/other/empty-elements-insertion.html

Line 46, Patchset 2 (Latest):

    const range = document.createRange();
    const selection = window.getSelection();
    range.selectNodeContents(target);
    range.collapse(false);
    const textNode = document.createTextNode(text);
    range.insertNode(textNode);
    selection.removeAllRanges();
    selection.addRange(range);
    assert_greater_than( target.innerHTML.length, 0, 'The text should be inserted into the styled <strong> element' );

Kent Tamura . resolved

How about adding a helper function for this?

Open in Gerrit

Related details

Attention is currently required from:

Dan Clark
Mason Freed
Pranav Modi

Sambamurthy Bandaru
Sanket Joshi
Tanu Jain

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Pranav Modi (Gerrit)

unread,

Mar 6, 2025, 4:28:38 AM3/6/25

to Kent Tamura, Mason Freed, Utkarsh Pathak, Tanu Jain, Sambamurthy Bandaru, Dan Clark, Sanket Joshi, Rakesh Goulikar, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Dan Clark, Kent Tamura, Mason Freed, Sambamurthy Bandaru, Sanket Joshi, Tanu Jain and Utkarsh Pathak

Pranav Modi added 1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Pranav Modi . resolved

@tk...@chromium.org, @mas...@chromium.org, Can you please review?

Open in Gerrit

Related details

Attention is currently required from:

Dan Clark
Kent Tamura
Mason Freed

Sambamurthy Bandaru
Sanket Joshi
Tanu Jain

Utkarsh Pathak

Submit Requirements:

Code-Coverage
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Pranav Modi (Gerrit)

unread,

Mar 6, 2025, 4:29:06 AM3/6/25

to Kent Tamura, Mason Freed, Utkarsh Pathak, Tanu Jain, Sambamurthy Bandaru, Dan Clark, Sanket Joshi, Rakesh Goulikar, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Dan Clark, Kent Tamura, Mason Freed, Sambamurthy Bandaru, Sanket Joshi, Tanu Jain and Utkarsh Pathak

Pranav Modi added 1 comment

File third_party/blink/web_tests/external/wpt/editing/other/empty-elements-insertion.html

Line 46, Patchset 2: const range = document.createRange();


    const selection = window.getSelection();
    range.selectNodeContents(target);
    range.collapse(false);
    const textNode = document.createTextNode(text);
    range.insertNode(textNode);
    selection.removeAllRanges();
    selection.addRange(range);
    assert_greater_than( target.innerHTML.length, 0, 'The text should be inserted into the styled <strong> element' );

Kent Tamura . resolved

How about adding a helper function for this?

Pranav Modi

Added!

satisfied_requirement

unsatisfied_requirement

open

diffy

Tanu Jain (Gerrit)

unread,

Mar 6, 2025, 10:50:20 AM3/6/25

to Pranav Modi, Kent Tamura, Mason Freed, Utkarsh Pathak, Sambamurthy Bandaru, Dan Clark, Sanket Joshi, Rakesh Goulikar, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Dan Clark, Kent Tamura, Mason Freed, Pranav Modi, Sambamurthy Bandaru, Sanket Joshi and Utkarsh Pathak

Tanu Jain voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Dan Clark
Kent Tamura
Mason Freed

Pranav Modi
Sambamurthy Bandaru
Sanket Joshi

Utkarsh Pathak

satisfied_requirement

unsatisfied_requirement

open

diffy

Dan Clark (Gerrit)

unread,

Mar 6, 2025, 2:07:30 PM3/6/25

to Pranav Modi, Tanu Jain, Kent Tamura, Mason Freed, Utkarsh Pathak, Sambamurthy Bandaru, Sanket Joshi, Rakesh Goulikar, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Kent Tamura, Mason Freed, Pranav Modi, Sambamurthy Bandaru, Sanket Joshi and Utkarsh Pathak

Dan Clark added 1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Dan Clark . resolved

Since these tests are supposed to be checking editing scenarios, it seems like it's going against the point if we replace them with DOM commands like inserting via Range.
Could we instead use https://web-platform-tests.org/writing-tests/testdriver.html#test_driver.send_keys, or this helper function: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/editing/include/editor-test-utils.js;l=38;drc=814df61549a8cc15c39ba6870a329aa324b50acf

That gets us closer to testing the actual user scenario of typing characters.

Open in Gerrit

Related details

Attention is currently required from:

satisfied_requirement

unsatisfied_requirement

open

diffy

Mason Freed (Gerrit)

unread,

Mar 6, 2025, 6:46:13 PM3/6/25

to Pranav Modi, Tanu Jain, Kent Tamura, Utkarsh Pathak, Sambamurthy Bandaru, Dan Clark, Sanket Joshi, Rakesh Goulikar, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Kent Tamura, Pranav Modi, Sambamurthy Bandaru, Sanket Joshi and Utkarsh Pathak

Mason Freed added 2 comments

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Dan Clark . unresolved

Since these tests are supposed to be checking editing scenarios, it seems like it's going against the point if we replace them with DOM commands like inserting via Range.
Could we instead use https://web-platform-tests.org/writing-tests/testdriver.html#test_driver.send_keys, or this helper function: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/editing/include/editor-test-utils.js;l=38;drc=814df61549a8cc15c39ba6870a329aa324b50acf
That gets us closer to testing the actual user scenario of typing characters.

Mason Freed

+1

Commit Message

Line 9, Patchset 3 (Latest):

The test failed on headless_shell because document.execCommand() is
returning false for insertText commands, causing the expected and

Mason Freed . unresolved

Is this expected? I believe `execCommand('insertText'` should be supported, so I'd expect it to work on headless also?

Open in Gerrit

Related details

Attention is currently required from:

Kent Tamura

Pranav Modi
Sambamurthy Bandaru
Sanket Joshi
Utkarsh Pathak

Submit Requirements:

Code-Coverage
Code-Review

No-Unresolved-Comments

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Pranav Modi (Gerrit)

unread,

Mar 28, 2025, 4:58:52 AM3/28/25

to Tanu Jain, Kent Tamura, Mason Freed, Sambamurthy Bandaru, Dan Clark, Rakesh Goulikar, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Dan Clark, Kent Tamura, Mason Freed, Sambamurthy Bandaru and Tanu Jain

Pranav Modi added 1 comment

Commit Message

Line 9, Patchset 3 (Latest):

The test failed on headless_shell because document.execCommand() is
returning false for insertText commands, causing the expected and

Mason Freed . resolved

Is this expected? I believe `execCommand('insertText'` should be supported, so I'd expect it to work on headless also?

Pranav Modi

In the link-boundaries-insertion.html test, when we run the execCommand after placing the selection, the text is supposed to be inserted inside the anchor tag, but it ends up inside the paragraph tag instead. So, the execCommand is working but inserting at wrong position. This seems to be a bug.

For another test, where the caret is supposed to be placed in an empty selection and then insert the letter 'a', it fails to place the caret in a non-styled strong element because there's no layout. For styled elements, the caret gets placed because the style gives the element a defined width and height, allowing the text to be inserted. This issue occurs in all browsers with the click event. As per the test, it tries to click at the position of empty selection. But, if only selection is placed using range and selection API's and insertText is called, firefox behaves properly and inserts 'a' unlike webkit and chromium.

Open in Gerrit

Related details

Attention is currently required from:

Dan Clark
Kent Tamura
Mason Freed

Sambamurthy Bandaru
Tanu Jain

Submit Requirements:

Code-Coverage
Code-Review
No-Unresolved-Comments

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Mason Freed (Gerrit)

unread,

Mar 31, 2025, 7:10:12 PM3/31/25

to Pranav Modi, Tanu Jain, Kent Tamura, Sambamurthy Bandaru, Dan Clark, Rakesh Goulikar, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Dan Clark, Kent Tamura, Pranav Modi, Sambamurthy Bandaru and Tanu Jain

Mason Freed added 1 comment

Commit Message

Line 9, Patchset 3 (Latest):

The test failed on headless_shell because document.execCommand() is
returning false for insertText commands, causing the expected and

Mason Freed . unresolved

Is this expected? I believe `execCommand('insertText'` should be supported, so I'd expect it to work on headless also?

Pranav Modi

In the link-boundaries-insertion.html test, when we run the execCommand after placing the selection, the text is supposed to be inserted inside the anchor tag, but it ends up inside the paragraph tag instead. So, the execCommand is working but inserting at wrong position. This seems to be a bug.
For another test, where the caret is supposed to be placed in an empty selection and then insert the letter 'a', it fails to place the caret in a non-styled strong element because there's no layout. For styled elements, the caret gets placed because the style gives the element a defined width and height, allowing the text to be inserted. This issue occurs in all browsers with the click event. As per the test, it tries to click at the position of empty selection. But, if only selection is placed using range and selection API's and insertText is called, firefox behaves properly and inserts 'a' unlike webkit and chromium.

Mason Freed

In the link-boundaries-insertion.html test, when we run the execCommand after placing the selection, the text is supposed to be inserted inside the anchor tag, but it ends up inside the paragraph tag instead. So, the execCommand is working but inserting at wrong position. This seems to be a bug.

Do we understand this bug? I'd like to avoid papering over a bug by changing the test to avoid it. It sounds like you've got a lot of context about what's going on, perhaps you could see why this bug exists? If not, I think it'd at least be a good idea to file a bug with these details so that we don't lose the context.

Open in Gerrit

Related details

Attention is currently required from:

Dan Clark
Kent Tamura
Pranav Modi
Sambamurthy Bandaru
Tanu Jain

satisfied_requirement

unsatisfied_requirement

open

diffy

Pranav Modi (Gerrit)

unread,

Apr 8, 2025, 4:01:17 PM4/8/25

to Tanu Jain, Kent Tamura, Mason Freed, Sambamurthy Bandaru, Dan Clark, Rakesh Goulikar, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Dan Clark, Kent Tamura, Mason Freed, Sambamurthy Bandaru and Tanu Jain

Pranav Modi added 1 comment

Commit Message

Line 9, Patchset 3 (Latest):

The test failed on headless_shell because document.execCommand() is
returning false for insertText commands, causing the expected and

Mason Freed . resolved

Is this expected? I believe `execCommand('insertText'` should be supported, so I'd expect it to work on headless also?

Pranav Modi

In the link-boundaries-insertion.html test, when we run the execCommand after placing the selection, the text is supposed to be inserted inside the anchor tag, but it ends up inside the paragraph tag instead. So, the execCommand is working but inserting at wrong position. This seems to be a bug.
For another test, where the caret is supposed to be placed in an empty selection and then insert the letter 'a', it fails to place the caret in a non-styled strong element because there's no layout. For styled elements, the caret gets placed because the style gives the element a defined width and height, allowing the text to be inserted. This issue occurs in all browsers with the click event. As per the test, it tries to click at the position of empty selection. But, if only selection is placed using range and selection API's and insertText is called, firefox behaves properly and inserts 'a' unlike webkit and chromium.

Mason Freed

In the link-boundaries-insertion.html test, when we run the execCommand after placing the selection, the text is supposed to be inserted inside the anchor tag, but it ends up inside the paragraph tag instead. So, the execCommand is working but inserting at wrong position. This seems to be a bug.
Do we understand this bug? I'd like to avoid papering over a bug by changing the test to avoid it. It sounds like you've got a lot of context about what's going on, perhaps you could see why this bug exists? If not, I think it'd at least be a good idea to file a bug with these details so that we don't lose the context.

Pranav Modi

Raised a bug with the link - https://crbug.com/409355663 to track.

Open in Gerrit

Related details

Attention is currently required from:

Dan Clark
Kent Tamura
Mason Freed
Sambamurthy Bandaru
Tanu Jain

satisfied_requirement

unsatisfied_requirement

open

diffy

Pranav Modi (Gerrit)

unread,

5:29 AM (2 hours ago) 5:29 AM

to Mike Jackson, Tanu Jain, Kent Tamura, Mason Freed, Sambamurthy Bandaru, Dan Clark, Rakesh Goulikar, Chromium LUCI CQ, chromium...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Pranav Modi abandoned this change.

View Change

Abandoned

Pranav Modi abandoned this change

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Review

No-Unresolved-Comments

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[WPT] Fix WPTs failing after switching to headless_shell [chromium/src : main]

Pranav Modi (Gerrit)

Pranav Modi added 1 comment

Related details

Pranav Modi (Gerrit)

Pranav Modi added 1 comment

Related details

Utkarsh Pathak (Gerrit)

Utkarsh Pathak voted Code-Review+1

Related details

Pranav Modi (Gerrit)

Pranav Modi added 1 comment

Related details

Kent Tamura (Gerrit)

Kent Tamura voted and added 1 comment

Votes added by Kent Tamura

1 comment

Related details

Pranav Modi (Gerrit)

Pranav Modi added 1 comment

Related details

Pranav Modi (Gerrit)

Pranav Modi added 1 comment

Tanu Jain (Gerrit)

Tanu Jain voted Code-Review+1

Related details

Dan Clark (Gerrit)

Dan Clark added 1 comment

Related details

Mason Freed (Gerrit)

Mason Freed added 2 comments

Related details

Pranav Modi (Gerrit)

Pranav Modi added 1 comment

Related details

Mason Freed (Gerrit)

Mason Freed added 1 comment

Related details

Pranav Modi (Gerrit)

Pranav Modi added 1 comment

Related details

Pranav Modi (Gerrit)

Pranav Modi abandoned this change

Related details