Closed Bug 1675883 Opened 4 years ago Closed 4 years ago

Modifying selectionStart fires double selection change events

Categories

(Core :: DOM: Selection, defect, P3)

defect

Tracking

()

RESOLVED FIXED
85 Branch
Tracking Status
firefox85 --- fixed

People

(Reporter: saschanaz, Assigned: saschanaz)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

Attached file selectionchange-double.html (deleted) —
  1. Open the attachment
  2. Open DevTools
  3. Wait until the cursor reaches the end

Expected: The log shows one event at a time and stops when the cursor reaches the end
Actual: It shows two events at a time and continues after reaching the end

Component: DOM: Core & HTML → DOM: Selection

This is happening from nsTextControlFrame::SetSelectionInternal where it calls Selection::RemoveAllRanges and then Selection::AddRangeAndSelectFramesAndNotifyListeners.

Relevant stack:

mozilla::SelectionChangeEventDispatcher::OnSelectionChange(mozilla::dom::Document * aDoc, mozilla::dom::Selection * aSel, short aReason) Line 87
mozilla::dom::Selection::NotifySelectionListeners() Line 3156
mozilla::dom::Selection::AddRangeAndSelectFramesAndNotifyListeners(nsRange & aRange, mozilla::dom::Document * aDocument, mozilla::ErrorResult & aRv) Line 1950
mozilla::dom::Selection::AddRangeAndSelectFramesAndNotifyListeners(nsRange & aRange, mozilla::ErrorResult & aRv) Line 1881
nsTextControlFrame::SetSelectionInternal(nsINode * aStartNode, unsigned int aStartOffset, nsINode * aEndNode, unsigned int aEndOffset, nsITextControlFrame::SelectionDirection aDirection) Line 841
nsTextControlFrame::SetSelectionEndPoints(unsigned int aSelStart, unsigned int aSelEnd, nsITextControlFrame::SelectionDirection aDirection) Line 941
nsTextControlFrame::SetSelectionRange(unsigned int aSelStart, unsigned int aSelEnd, nsITextControlFrame::SelectionDirection aDirection) Line 959
mozilla::TextControlState::SetSelectionRange(unsigned int aStart, unsigned int aEnd, nsITextControlFrame::SelectionDirection aDirection, mozilla::ErrorResult & aRv) Line 2101
mozilla::TextControlState::SetSelectionStart(const mozilla::dom::Nullable<unsigned int> & aStart, mozilla::ErrorResult & aRv) Line 2157
mozilla::dom::HTMLInputElement::SetSelectionStart(const mozilla::dom::Nullable<unsigned int> & aSelectionStart, mozilla::ErrorResult & aRv) Line 5449
Assignee: nobody → krosylight
Status: NEW → ASSIGNED
Severity: -- → S3
Priority: -- → P3
Pushed by krosylight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5ff33e73771f Use SetStartAndEndInLimiter() in SetSelectionInternal() r=masayuki
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/26505 for changes under testing/web-platform/tests

Backed out changeset 5ff33e73771f (bug 1675883) for browser_editcontrols_update.js failures.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&searchStr=os%2Cx%2C10.14%2Cwebrender%2Cmochitests%2Cbc&fromchange=a4bbd54841f1c2c3ac8f352cb64c158b02a8dc0e&tochange=ad4013a9048a702ea32c8d69b2888a5ef191e1a3&selectedTaskRun=BdDvdnLJRX64sDuuaRpXAQ.0

Backout link: https://hg.mozilla.org/integration/autoland/rev/ad4013a9048a702ea32c8d69b2888a5ef191e1a3

Failure log: https://treeherder.mozilla.org/logviewer?job_id=321595802&repo=autoland&lineNumber=2076

[task 2020-11-12T19:02:42.188Z] 19:02:42     INFO - TEST-START | browser/components/customizableui/test/browser_editcontrols_update.js
[task 2020-11-12T19:03:27.325Z] 19:03:27     INFO - TEST-INFO | started process screencapture
[task 2020-11-12T19:03:27.385Z] 19:03:27     INFO - TEST-INFO | screencapture: exit 0
[task 2020-11-12T19:03:27.385Z] 19:03:27     INFO - Buffered messages logged at 19:02:42
[task 2020-11-12T19:03:27.386Z] 19:03:27     INFO - Entering test bound test_init
[task 2020-11-12T19:03:27.386Z] 19:03:27     INFO - Initializing clipboard with "waitForClipboard-known-value-0.6738718010771225"...
[task 2020-11-12T19:03:27.386Z] 19:03:27     INFO - Succeeded initializing clipboard, start requested things...
[task 2020-11-12T19:03:27.386Z] 19:03:27     INFO - TEST-PASS | browser/components/customizableui/test/browser_editcontrols_update.js | Clipboard has the given value: 'Sample' - 
[task 2020-11-12T19:03:27.392Z] 19:03:27     INFO - TEST-PASS | browser/components/customizableui/test/browser_editcontrols_update.js | The panel is closed to begin with. - "closed" == "closed" - 
[task 2020-11-12T19:03:27.393Z] 19:03:27     INFO - TEST-PASS | browser/components/customizableui/test/browser_editcontrols_update.js | The panel is open to begin with. - true == true - 
[task 2020-11-12T19:03:27.393Z] 19:03:27     INFO - Leaving test bound test_init
[task 2020-11-12T19:03:27.393Z] 19:03:27     INFO - Entering test bound test_panelui_opened
[task 2020-11-12T19:03:27.393Z] 19:03:27     INFO - TEST-PASS | browser/components/customizableui/test/browser_editcontrols_update.js | The panel is closed to begin with. - "closed" == "closed" - 
[task 2020-11-12T19:03:27.393Z] 19:03:27     INFO - TEST-PASS | browser/components/customizableui/test/browser_editcontrols_update.js | Update when edit-controls is on panel and visible - cut - 
[task 2020-11-12T19:03:27.393Z] 19:03:27     INFO - TEST-PASS | browser/components/customizableui/test/browser_editcontrols_update.js | Update when edit-controls is on panel and visible - paste - 
[task 2020-11-12T19:03:27.394Z] 19:03:27     INFO - TEST-PASS | browser/components/customizableui/test/browser_editcontrols_update.js | Update when edit-controls is on panel and selection changed - cut - 
[task 2020-11-12T19:03:27.394Z] 19:03:27     INFO - TEST-PASS | browser/components/customizableui/test/browser_editcontrols_update.js | Update when edit-controls is on panel and selection changed - paste - 
[task 2020-11-12T19:03:27.394Z] 19:03:27     INFO - TEST-PASS | browser/components/customizableui/test/browser_editcontrols_update.js | The panel is open to begin with. - true == true - 
[task 2020-11-12T19:03:27.394Z] 19:03:27     INFO - TEST-PASS | browser/components/customizableui/test/browser_editcontrols_update.js | Update when edit-controls is on panel and hidden - cut - 
[task 2020-11-12T19:03:27.394Z] 19:03:27     INFO - TEST-PASS | browser/components/customizableui/test/browser_editcontrols_update.js | Update when edit-controls is on panel and hidden - paste - 
[task 2020-11-12T19:03:27.394Z] 19:03:27     INFO - Buffered messages finished
[task 2020-11-12T19:03:27.395Z] 19:03:27     INFO - TEST-UNEXPECTED-FAIL | browser/components/customizableui/test/browser_editcontrols_update.js | Test timed out - 
[task 2020-11-12T19:03:27.395Z] 19:03:27     INFO - GECKO(2214) | MEMORY STAT | vsize 8114MB | residentFast 485MB | heapAllocated 124MB
[task 2020-11-12T19:03:27.395Z] 19:03:27     INFO - TEST-OK | browser/components/customizableui/test/browser_editcontrols_update.js | took 45152ms
Flags: needinfo?(krosylight)
Upstream PR was closed without merging
Flags: needinfo?(krosylight)

It's weird, nothing should be depending on the previous selectionchange event behavior because it's a Nightly-only thing, and select event behavior is not affected so it shouldn't cause any bug here (although it turns out it's also flawed, with many WPT failures). It's hard to debug those failures as they are macOS specific and I have no Mac.

Gijs, I see your name on the commit log, do you have any potential cause in mind from the current URL bar implementation? Could you try applying my patch if your time allows it?

The intention here is to fire selectionchange events only when there is an actual selection range change (so that it can only fire once after repeated select() call).

Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Kagami :saschanaz from comment #7)

It's weird, nothing should be depending on the previous selectionchange event behavior because it's a Nightly-only thing, and select event behavior is not affected so it shouldn't cause any bug here (although it turns out it's also flawed, with many WPT failures). It's hard to debug those failures as they are macOS specific and I have no Mac.

Try a one-click loaner, or have IT send you a loaner? Seems unfortunate not to be able to debug things.

Gijs, I see your name on the commit log, do you have any potential cause in mind from the current URL bar implementation? Could you try applying my patch if your time allows it?

I'm afraid I don't have time to apply the patch, build, and then debug what's going on. But if I had to guess, the test calls gURLBar.select() and it sounds like behaviour there might have changed, cf. https://searchfox.org/mozilla-central/rev/c37038c592a352eda0f5e77dfb58c4929bf8bcd3/browser/components/urlbar/UrlbarInput.jsm#298,308-313 ?

The test then waits for a XUL command controller update, which...

The intention here is to fire selectionchange events only when there is an actual selection range change (so that it can only fire once after repeated select() call).

... it sounds like this might inhibit?

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(krosylight)
Attachment #9188477 - Attachment description: Bug 1675883 - Update edit control test to match the change r=gijs → Bug 1675883 - Update edit control test to match the select event change r=gijs
Flags: needinfo?(krosylight)
Attachment #9188477 - Attachment description: Bug 1675883 - Update edit control test to match the select event change r=gijs → Bug 1675883 - Update edit control test to match the change r=gijs
Pushed by krosylight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/60d38023577e Use SetStartAndEndInLimiter() in SetSelectionInternal() r=masayuki https://hg.mozilla.org/integration/autoland/rev/7942e7b546c3 Update edit control test to match the change r=Gijs
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 85 Branch
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: