This bug is a follow-up of bug 1020801. We should update the title once bug 1020801 is merged and identify what should be really fixed here.

Here's what I found when testing the patch from bug 1020801 STR: 1. long press some characters, and it will show dialog, and remove finger 2. touch the screen again, and start to scroll 3. stop scrolling and remove finger Log between step 1 and 2 is as below, E/GeckoConsole( 292): Content JS LOG at app://system.gaiamobile.org/js/text_selection_dialog.js:199 in tsd_show: {"rect":{"width":58.66667175292969,"height":14.666671752929688,"top":76.33332824707031,"bottom":91,"left":112.33332824707031,"right":171},"commands":{"canSelectAll":true,"canCut":true,"canCopy":true,"canPaste":false},"zoomFactor":1,"reasons":["mouseup"],"isCollapsed":false,"msg_name":"selectionchange","offsetX":0,"offsetY":30} Log between step 2 and 3 is as below, E/GeckoConsole( 292): Content JS LOG at app://system.gaiamobile.org/js/text_selection_dialog.js:74 in tsd_handleEvent: selectionchange E/GeckoConsole( 292): Content JS LOG at app://system.gaiamobile.org/js/text_selection_dialog.js:199 in tsd_show: {"rect":{"width":320,"height":303.33331298828125,"top":30,"bottom":333.33331298828125,"left":0,"right":320},"commands":{"canSelectAll":true,"canCut":false,"canCopy":true,"canPaste":false},"zoomFactor":1,"reasons":["drag","mousedown"],"isCollapsed":true,"msg_name":"selectionchange","offsetX":0,"offsetY":0} E/GeckoConsole( 292): Content JS LOG at app://system.gaiamobile.org/js/text_selection_dialog.js:74 in tsd_handleEvent: selectionchange E/GeckoConsole( 292): Content JS LOG at app://system.gaiamobile.org/js/text_selection_dialog.js:199 in tsd_show: {"rect":{"width":320,"height":303.33331298828125,"top":30,"bottom":333.33331298828125,"left":0,"right":320},"commands":{"canSelectAll":true,"canCut":false,"canCopy":true,"canPaste":false},"zoomFactor":1,"reasons":[],"isCollapsed":true,"msg_name":"selectionchange","offsetX":0,"offsetY":0} E/GeckoConsole( 292): Content JS LOG at app://system.gaiamobile.org/js/text_selection_dialog.js:74 in tsd_handleEvent: scrollviewchange 1. the value of isCollapsed are different "isCollapsed":false "isCollapsed":true I believe it should be always true, since the selected area I saw on the screen is not collapsed after scrolling. 2. commands are different {"canSelectAll":true,"canCut":true,"canCopy":true,"canPaste":false} {"canSelectAll":true,"canCut":false,"canCopy":true,"canPaste":false} I think they should be the same, or some button might be not workable after scrolling 3. rects are different {"rect":{"width":58.66667175292969,"height":14.666671752929688,"top":76.33332824707031,"bottom":91,"left":112.33332824707031,"right":171} {"rect":{"width":320,"height":303.33331298828125,"top":30,"bottom":333.33331298828125,"left":0,"right":320} with and height should be the same due to same selected area. and top/bottom/left/right should changed by scrolling. 4. tsd_handleEvent: scrollviewchange happen after selectionchange It makes text_seleciton_dialog.js updated with incorrect message of above items. We should have way to filter the selectionchange event when user start to scroll and make sure the commands are the same scrolling.

And another problem is, evt.detail.scrollHeight should be yEnd - yStart, right? I try to scroll from the end of the text, and the scrollHeight is its position y, have I misunderstood something?

Based on comment 1, gecko sends out the following selectoinchange to gaia during the scrolling(before AsyncPanZoomStarted callback is received). I can filter this redundant selectionchange in my local but I will also check the source to trigger this selection change. [Log] 09-15 04:53:05.960 I/Gecko (17861): peter nsDocViewerSelectionListener::NotifySelectionChanged called 09-15 04:53:05.960 I/Gecko (17861): peter UpdateCommands rect 0 30 320 303.333 reason 0x00000003 09-15 04:53:05.970 I/Gecko (17861): peter send selectionchange event top 30 bottom 333.33331298828125 left 0 right 320 content.screen.width 320 innerWidth 320 content.screen.height 569 09-15 04:53:05.970 I/Gecko (17861): peter nsDocViewerSelectionListener::NotifySelectionChanged called 09-15 04:53:05.970 I/Gecko (17861): peter UpdateCommands rect 0 30 320 303.333 reason 0x00000000 09-15 04:53:05.970 I/Gecko (17861): peter send selectionchange event top 30 bottom 333.33331298828125 left 0 right 320 content.screen.width 320 innerWidth 320 content.screen.height 569 09-15 04:53:06.100 I/Gecko (18245): peter AsyncPanZoomStarted called 09-15 04:53:06.580 I/Gecko (18245): peter AsyncPanZoomStopped called 0 113 09-15 04:53:06.580 I/Gecko (18245): peter AsyncPanZoomStarted called 09-15 04:53:06.590 I/Gecko (18245): peter AsyncPanZoomStopped called 0 113

It is easy to see the selection change events with no reason, not matter content is scrolled or not.

There is another performance impaction that was caused by too many selection change events. https://bugzilla.mozilla.org/show_bug.cgi?id=1066515#c43

Bypass the selection events with no reason and keypress reason to system app to avoid performance regression.

Comment on attachment 8492984 [details] [diff] [review] bug_1065955.patch Review of attachment 8492984 [details] [diff] [review]: ----------------------------------------------------------------- Instead of hacking something in like this, we should figure out where these events are coming from, and deciding whether we actually want to ignore them. For example, I don't think ignoring selection changes through key events is the right thing to do.

(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #7) > Comment on attachment 8492984 [details] [diff] [review] > bug_1065955.patch > > Review of attachment 8492984 [details] [diff] [review]: > ----------------------------------------------------------------- > > Instead of hacking something in like this, we should figure out where these > events are coming from, and deciding whether we actually want to ignore > them. For example, I don't think ignoring selection changes through key > events is the right thing to do. Ehsan, there are lots of selection changes event with NO_REASON and KEYPRESS_REASON when choosing the word suggestion from keyboard app in bug 1066515. For the events with keypress reason, they are triggered from [1] and the keypress reason are appended inside nsFrameSelection::MoveCaret. For FxOS or fennec, I think it is impossible for users to trigger nsFrameSelection::MoveCaret but you can use script to trigger, besides, current the management of selection bubble in gaia doesn't handle the keypress reason. [1]http://mxr.mozilla.org/mozilla-central/source/dom/inputmethod/forms.js?rev=ae2e4ca015fd#1053 [2]http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsSelection.cpp?rev=ae2e4ca015fd#926

(In reply to peter chang[:pchang][:peter] from comment #8) > (In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from > comment #7) > > Comment on attachment 8492984 [details] [diff] [review] > > bug_1065955.patch > > > > Review of attachment 8492984 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > Instead of hacking something in like this, we should figure out where these > > events are coming from, and deciding whether we actually want to ignore > > them. For example, I don't think ignoring selection changes through key > > events is the right thing to do. > > Ehsan, there are lots of selection changes event with NO_REASON and > KEYPRESS_REASON when choosing the word suggestion from keyboard app in bug > 1066515. For the events with keypress reason, they are triggered from [1] > and the keypress reason are appended inside nsFrameSelection::MoveCaret. Thanks! So, this is an API exposed to Web content allowing it to extend the selection. Your patch breaks updating the selection handles when a web page does this, so it is not acceptable. That being said, I don't understand why the forms.js code does things that way. A much better way which would avoid all of these intermediate events would be something like: sel.collapse(node, 0); sel.extend(node, start); or some such. > For FxOS or fennec, I think it is impossible for users to trigger > nsFrameSelection::MoveCaret That function can be called from the Selection.extend method. > but you can use script to trigger, besides, > current the management of selection bubble in gaia doesn't handle the > keypress reason. Then that is a bug that we should fix as well.

Bug 1066515 was landed but I still could reproduce the low performance issue from message app. When the low performance happened, the CPU usage of b2g and message app were very high. And I saw a lot of handleSelectionChange in BrowserElementParent.jsm. Checking why so many handleSelectionChange happened. User 60%, System 18%, IOW 0%, IRQ 0% User 84 + Nice 52 + Sys 42 + Idle 47 + IOW 0 + IRQ 0 + SIRQ 1 = 226 PID TID PR CPU% S VSS RSS PCY UID Thread Proc 19835 19835 1 33% R 230868K 85080K root b2g /system/b2g/b2g 20275 20275 1 24% R 117152K 35944K u0_a2027 Messages /system/b2g/b2g 20479 20479 0 5% R 1416K 656K root top top 279 279 0 3% S 5620K 240K root adbd /sbin/adbd 10-25 22:48:53.662 19835 19835 I Gecko : peter NotifySelectionChange aReason 0x00000003 10-25 22:48:53.662 19835 19835 I Gecko : peter NotifySelectionChange aReason 0x00000000 10-25 22:48:53.662 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event 10-25 22:48:53.662 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event 10-25 22:48:53.672 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange finish 10-25 22:48:53.682 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event 10-25 22:48:53.682 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event 10-25 22:48:53.692 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange finish 10-25 22:48:53.782 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event 10-25 22:48:53.782 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event 10-25 22:48:53.792 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange finish 10-25 22:48:53.802 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event 10-25 22:48:53.802 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event ... 10-25 22:48:55.432 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event 10-25 22:48:55.432 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event 10-25 22:48:55.442 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange finish 10-25 22:48:55.442 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event 10-25 22:48:55.442 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event 10-25 22:48:55.452 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange finish 10-25 22:48:55.452 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event 10-25 22:48:55.452 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event 10-25 22:48:55.452 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange finish 10-25 22:48:55.462 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event 10-25 22:48:55.462 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event 10-25 22:48:55.462 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange finish 10-25 22:48:55.462 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event 10-25 22:48:55.462 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event 10-25 22:48:55.472 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange finish 10-25 22:48:55.472 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event 10-25 22:48:55.472 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event 10-25 22:48:55.482 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange finish 10-25 22:48:55.482 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event 10-25 22:48:55.482 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event 10-25 22:48:55.482 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange finish 10-25 22:48:55.492 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event 10-25 22:48:55.492 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event ... 10-25 22:48:58.392 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event 10-25 22:48:58.392 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event 10-25 22:48:58.392 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange finish 10-25 22:48:58.392 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event 10-25 22:48:58.402 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event 10-25 22:48:58.402 20275 20275 I Gecko : peter NotifySelectionChange aReason 0x00000008 10-25 22:48:58.402 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange finish 10-25 22:48:58.402 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event 10-25 22:48:58.402 20275 20275 I Gecko : peter NotifySelectionChange aReason 0x00000000 10-25 22:48:58.402 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event 10-25 22:48:58.412 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange finish 10-25 22:48:58.412 19835 19835 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event 10-25 22:48:58.412 20275 20275 I Gecko : peter NotifySelectionChange aReason 0x00000008

BTW, I tried to use gecko profiler to check the bottleneck and also found many sample time for handleSelectionChange. Profiler for b2g main thread http://people.mozilla.org/~bgirard/cleopatra/#report=058905773c0c412d25b75c7b7171700a4fc6b32c

As I expected, the selection events in comment 10 were triggered with "no_resaon" and "keypress" reasons. 10-28 16:09:58.712 22265 22265 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event data{"rect":{"width":0,"height":16.666671752929688,"top":127.5,"bottom":144.1666717529297,"left":51,"right":51},"commands":{"canSelectAll":true,"canCut":false,"canCopy":false,"canPaste":false},"zoomFactor":1,"reasons":[],"isCollapsed":true,"msg_name":"selectionchange"} 10-28 16:09:58.712 22265 22265 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event 10-28 16:09:58.712 22265 22265 I Gecko : BrowserElementParent.jsm - handleSelectionChange finish 10-28 16:09:58.722 22265 22265 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to create event data{"rect":{"width":0,"height":16.666671752929688,"top":127.5,"bottom":144.1666717529297,"left":57,"right":57},"commands":{"canSelectAll":true,"canCut":false,"canCopy":false,"canPaste":false},"zoomFactor":1,"reasons":["keypress"],"isCollapsed":true,"msg_name":"selectionchange"} 10-28 16:09:58.722 22265 22265 I Gecko : BrowserElementParent.jsm - handleSelectionChange start to dispatch event 10-28 16:09:58.722 22265 22265 I Gecko : BrowserElementParent.jsm - handleSelectionChange finish

Instead of reducing the selection change events with no_reason and keypress, create this patch to filter the collapsed selection change events. With this patch, it doesn't cause performance degradation when using word suggestion.

(In reply to peter chang[:pchang][:peter] from comment #13) > Created attachment 8509332 [details] [diff] [review] > WIP v2 > > Instead of reducing the selection change events with no_reason and keypress, > create this patch to filter the collapsed selection change events. > > With this patch, it doesn't cause performance degradation when using word > suggestion. And this patch could reduce lots of selection change events in comment 10 and comment 12.

Peter, can we please discuss a proper way to fix this bug first? I hate to keep saying no to your patches after you've spent the time to write them... I don't think your approach is a good one. It is focused too much on fixing only one issue, and it's the wrong thing to do because it is crippling the selectionchange event for all other usages. What is the ultimate source of this bug? Do we still believe that the look in setSelectionRange inside forms.js is at fault here? If yes, is it possible to rewrite that code to not modify the selection in a loop like it does today? If not, I'd be fine with adding a hack which allows that code (and only that code) to suppress sending all of the intermediate selectionchange events, but deciding to not dispatch the events if the selection is collapsed and the touch caret is invisible is too big of a hammer here.

Comment on attachment 8509332 [details] [diff] [review] WIP v2 Clearing the request for now.

(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #15) > Peter, can we please discuss a proper way to fix this bug first? I hate to > keep saying no to your patches after you've spent the time to write them... > I think it's my fault, I should explain more detail about my patch. > I don't think your approach is a good one. It is focused too much on fixing > only one issue, and it's the wrong thing to do because it is crippling the > selectionchange event for all other usages. > > What is the ultimate source of this bug? Do we still believe that the look > in setSelectionRange inside forms.js is at fault here? If yes, is it > possible to rewrite that code to not modify the selection in a loop like it > does today? If not, I'd be fine with adding a hack which allows that code > (and only that code) to suppress sending all of the intermediate > selectionchange events, but deciding to not dispatch the events if the > selection is collapsed and the touch caret is invisible is too big of a > hammer here. With Bug 1066515, it adds the fast path to resolve original problem but it is easy to fall back to original path to hit the low performance problem[1]. Based on my measure in comment 10, the cost of sending DOM events cross IPC is not cheap. In my opinion, even we fix the code in form.js but there may be another similar use case to hit this low performance problem and we have to turn off selection caret. That's why I want to fix this problem in gecko side. Does it make sense to you? After re-checking the code in [2], I could modify attachment 8509332 [details] [diff] [review] not to dispatch the events only when selection collapse flag is not changed and touch caret is invisible. [1]http://dxr.mozilla.org/mozilla-central/source/dom/inputmethod/forms.js#1179 [2]http://dxr.mozilla.org/mozilla-central/source/layout/base/nsDocumentViewer.cpp#3559

(In reply to peter chang[:pchang][:peter] from comment #17) > With Bug 1066515, it adds the fast path to resolve original problem but it > is easy to fall back to original path to hit the low performance problem[1]. Oh, I wish I had reviewed that patch. :( So, I don't think that was the right approach either. As I said, given that we *know* that this is a performance problem in that code, we should just suppress sending the selection events in that code in general. We can surround that loop with a suppressSendingSelectionChangeEvents/unsuppressSendingSelectionChangeEvents calls (hopefully you can think of better function names ;) and in UpdateState, check a boolean condition based on that. > Based on my measure in comment 10, the cost of sending DOM events cross IPC > is not cheap. > In my opinion, even we fix the code in form.js but there may be another > similar use case to hit this low performance problem and we have to turn off > selection caret. > > That's why I want to fix this problem in gecko side. Does it make sense to > you? Well, sure. But the question is, should we cripple the event while doing so? That is what I would like us to avoid. > After re-checking the code in [2], I could modify attachment 8509332 [details] [diff] [review] > [details] [diff] [review] not to dispatch the events only when selection > collapse flag is not changed and touch caret is invisible. > > [1]http://dxr.mozilla.org/mozilla-central/source/dom/inputmethod/forms. > js#1179 > [2]http://dxr.mozilla.org/mozilla-central/source/layout/base/ > nsDocumentViewer.cpp#3559 The select event is used to update parts of the UI that should be activated if there is *any* non-collapsed selection, so in that sense, dispatching that event *only* when we go from collapsed to non-collapsed and vice versa makes sense. However, the selectionchange at least as I originally intended it is used for notifying about any changes to the selection. So if we for example want to build some kind of UI which should respond to actual selection changes, doing what you suggest will make this event useless for that purpose. But perhaps I'm stressing over the generality side too much. If you really think there's going to be 0 use cases for a generic selection change event, perhaps we should give up on making one for now. But I think we should at least rename the event to something that doesn't suggest that it works for selection changes...

(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #18) > (In reply to peter chang[:pchang][:peter] from comment #17) > > With Bug 1066515, it adds the fast path to resolve original problem but it > > is easy to fall back to original path to hit the low performance problem[1]. > > Oh, I wish I had reviewed that patch. :( > > So, I don't think that was the right approach either. As I said, given that > we *know* that this is a performance problem in that code, we should just > suppress sending the selection events in that code in general. We can > surround that loop with a > suppressSendingSelectionChangeEvents/unsuppressSendingSelectionChangeEvents > calls (hopefully you can think of better function names ;) and in > UpdateState, check a boolean condition based on that. > I think this suggestion is good and create bug 1089553 to follow up. > > Based on my measure in comment 10, the cost of sending DOM events cross IPC > > is not cheap. > > In my opinion, even we fix the code in form.js but there may be another > > similar use case to hit this low performance problem and we have to turn off > > selection caret. > > > > That's why I want to fix this problem in gecko side. Does it make sense to > > you? > > Well, sure. But the question is, should we cripple the event while doing > so? That is what I would like us to avoid. > > > After re-checking the code in [2], I could modify attachment 8509332 [details] [diff] [review] > > [details] [diff] [review] not to dispatch the events only when selection > > collapse flag is not changed and touch caret is invisible. > > In my local testing, it caused trobule if we only dispatch selection change events when collapse state changed because there are several selection events which are not collapsed but with different reasons. Copypaste bubble needs those events to display the bubble. > > [1]http://dxr.mozilla.org/mozilla-central/source/dom/inputmethod/forms. > > js#1179 > > [2]http://dxr.mozilla.org/mozilla-central/source/layout/base/ > > nsDocumentViewer.cpp#3559 > > The select event is used to update parts of the UI that should be activated > if there is *any* non-collapsed selection, so in that sense, dispatching > that event *only* when we go from collapsed to non-collapsed and vice versa > makes sense. However, the selectionchange at least as I originally intended > it is used for notifying about any changes to the selection. So if we for > example want to build some kind of UI which should respond to actual > selection changes, doing what you suggest will make this event useless for > that purpose. > > But perhaps I'm stressing over the generality side too much. If you really > think there's going to be 0 use cases for a generic selection change event, > perhaps we should give up on making one for now. But I think we should at > least rename the event to something that doesn't suggest that it works for > selection changes... Right now the 'selectionchange' event here is used to notify the copypaste bubble and it works fine if we filter the collapsed selection events. And I still perfer to prevent sending lots of selection change events in gecko to prevent another similar use case to hit the performance problem. As you mention above, the meaning of 'selectionchange' implies we need to notify the copypaste bubble for any selection changes. But actually the copypaste bubble is not interested at those collapsed events, they also have some event filter logic. Therefore, I suggest to change the 'selectionchange' event to 'copypaste' event. Since this modifcation requires lots of file changes, I would prefer to fix the naming issue in another. Ehsan, please let me know if you prefer to fix inside this bug.

OK, let's rename the event and tailor it to the copy/paste bubble use case. I'll leave it up to you which bug to do the work in. :-)

(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!, PTO 11/3-11/21) from comment #20) > OK, let's rename the event and tailor it to the copy/paste bubble use case. > I'll leave it up to you which bug to do the work in. :-) I just create the bug 1090008 to rename the event.

Filter collapsed selection change events but exclude the shortcut mode that the touch caret is visible. As I mention in comment 19, it caused some problem if we dispatch the selection chagne events when collapse state changed like select event[1] did. Therefore, the patch only targets at the collapsed selection change events. [1]http://dxr.mozilla.org/mozilla-central/source/layout/base/nsDocumentViewer.cpp?from=nsDocumentViewer.cpp#3559

Comment on attachment 8512412 [details] [diff] [review] WIP v3 Review of attachment 8512412 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsGlobalWindow.cpp @@ +9357,5 @@ > init.mBubbles = true; > if (aSel) { > + bool isCollapsed = false; > + bool isTouchCaretVisible = false; > + aSel->GetIsCollapsed(&isCollapsed); Nit: instead, do: bool isCollapsed = aSel->Collapsed();

Fix the nits. And the try result is positive. https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=1d1e9d6b3813