Closed Bug 1866431 Opened 5 months ago Closed 4 months ago

"WebDriver:ElementSendKeys" fails to send text that contains surrogate pairs

Categories

(Remote Protocol :: Marionette, defect, P2)

Firefox 118
defect

Tracking

(firefox-esr115 unaffected, firefox120 disabled, firefox121 disabled, firefox122 disabled, firefox123 fixed)

RESOLVED FIXED
123 Branch
Tracking Status
firefox-esr115 --- unaffected
firefox120 --- disabled
firefox121 --- disabled
firefox122 --- disabled
firefox123 --- fixed

People

(Reporter: whimboo, Assigned: jgraham)

References

(Regression)

Details

(Keywords: regression, Whiteboard: [webdriver:m9][webdriver:external], [wptsync upstream][webdriver:relnote])

Attachments

(3 files)

Originally filed as: https://github.com/mozilla/geckodriver/issues/2139

Note that this problem only occurs when running Nightly and early beta builds of Firefox. Using mozregression bug 1840519 has been revealed.

Note that this happens only in these builds of Firefox because there the preference dom.event.keypress.dispatch_once_per_surrogate_pair is set to false. If it's true the following Marionette testcase will pass:

    def test(self):
        self.marionette.set_pref(
            "dom.event.keypress.dispatch_once_per_surrogate_pair", False)

        self.marionette.navigate(
            "https://www.selenium.dev/selenium/web/cn-test.html")
        elem = self.marionette.find_element(By.NAME, "i18n")
      
        text = "𠀀𠜎𠀋𪆐𪚲"
        elem.send_keys(text)
        self.assertEqual(elem.get_property("value"), text)

Masayuki, can you please take a look? For now I filed it as a Marionette bug but I assume it's somewhere in DOMWindowUtils or platform code?

Flags: needinfo?(masayuki)

This testcase can easily be run after applying with the following command:

./mach marionette-test --headless -vv --gecko-log - --setpref="remote.log.truncate=true" testing/marionette/harness/marionette_harness/tests/unit/test_typing.py

I guess that the Marionette synthesizes different style of keypress events.

It seems that here splits surrogate pairs to surrogate:
https://searchfox.org/mozilla-central/rev/edb2612db13e89f1c44ab95b1e4d4366c16eb9fb/testing/marionette/client/marionette_driver/marionette.py#188-190

However, I'm not familiar with Python and its string so that I'm not 100% sure whether here is buggy, and I'm not sure whether Python can check whether a surrogate or not for each "character" smoothly.

Another possible fix point (if here splits surrogate pairs) is, at the receiver side. If it gets a high-surrogate keydown and/or keyup event, make it wait for the following keydown and/or keyup for a low-surrogate. Then, they can synthesize a set of keys with checking the pref. I guess that this is better solution, but I've not found the receiver side. Could you let me know where is it?

(I.e., a surrogate pair will synthesize a set of keydown and keyup with one or two keypress events once the new behavior will be shipped and that's what same behavior as user input at least on Windows.)

And what do you think for this comment from your point of view?

Flags: needinfo?(masayuki) → needinfo?(hskupin)

Discussed in todays triage meeting and James will have a look. If it's a problem with Marionette we might turn off this new feature for now until we got it correctly implemented in Marionette.

Flags: needinfo?(hskupin) → needinfo?(james)

Just two additional maybe related issues that I noticed:

This is mostly reusing the implementation from the marionette client.

Assignee: nobody → james
Status: NEW → ASSIGNED

Previously, given a string in element send keys, we iterated over each
UCS2 "character" in the string, so that non-BMP characters ended up as
separate key events, one for the high surrogate and one for the low
surrogate.

With this change we instead use the built in string iterator which
provides the surrogate pair in the case of non-BMP characters, so we
emit a single event for each unicode codepoint.

Flags: needinfo?(james)
Severity: -- → S3
Priority: -- → P2
Whiteboard: [webdriver:m9][webdriver:external]
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/autoland/rev/b2886b896d0d
Fixture for setting prefs in mozilla-only wdspec tests, r=webdriver-reviewers,whimboo

The second patch also needs to be pushed. As such lets mark as leave-open for now.

Keywords: leave-open
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/autoland/rev/f19c3570bc57
Send full codepoints when creating key events, r=webdriver-reviewers,masayuki,whimboo
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/43722 for changes under testing/web-platform/tests
Whiteboard: [webdriver:m9][webdriver:external] → [webdriver:m9][webdriver:external], [wptsync upstream]
Keywords: leave-open
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 123 Branch
Upstream PR merged by moz-wptsync-bot

The patch landed in nightly and beta is affected.
:jgraham, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox122 to wontfix.

For more information, please visit BugBot documentation.

Flags: needinfo?(james)

The fix here on this bug is actually for a feature that is only enabled in Firefox Nightly. So beta is not affected.

Flags: needinfo?(james)
Whiteboard: [webdriver:m9][webdriver:external], [wptsync upstream] → [webdriver:m9][webdriver:external], [wptsync upstream][webdriver:relnote]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: