Since non-editable fields selection is ready but behind the pref. We'll enable it by default in this bug.

Remove the pref. try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ba33551c24ba

Accidentally import wrong patches on previous try run. new try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=789fc662e394

Remove non-editable preference because we don't need it anymore.

In previous try, we crash at SetDragState because null FrameSelection. So I add check before use.

Disable selection carets in those fail testcases.

try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=2052877ce0ee

Although all patches are r+ and try looks good. We get too much noise from system app. So I'll check in this patch after bug 1092425 land.

patch failed to apply: renamed 1092888 -> Bug-1092888---Part-1-Remove-preference-for-selecti.patch applying Bug-1092888---Part-1-Remove-preference-for-selecti.patch patching file layout/base/SelectionCarets.cpp Hunk #1 FAILED at 56 Hunk #2 FAILED at 95 2 out of 4 hunks FAILED -- saving rejects to file layout/base/SelectionCarets.cpp.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir errors during apply, please fix and refresh Bug-1092888---Part-1-Remove-preference-for-selecti.patch could you take a look, thanks!

rebased.

rebased.

Patches has been rebased. Sorry about it! Please check-in again, thanks.

https://hg.mozilla.org/integration/mozilla-inbound/rev/e4e735cdd665 https://hg.mozilla.org/integration/mozilla-inbound/rev/c25a6fd7903f https://hg.mozilla.org/integration/mozilla-inbound/rev/eeff2ff03d39

All backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/2f5bd2f62b7a for Marionette test bustage: https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3742926&repo=mozilla-inbound

The Marionette failures appear to be all on Windows builds, if that matters.

Non-editable selection is work well, so we need flip the assert funtion in existing test for non-editable test. try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=933eef2e1609

Comment on attachment 8520358 [details] [diff] [review] Part 4: Flip testing function for non-editablt test at test_selectioncarets.py. Review of attachment 8520358 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/tests/marionette/test_selectioncarets.py @@ +215,5 @@ > # <div> non-editable test cases with selection carets enabled > ######################################################################## > def test_content_non_editable_minimum_select_one_character_by_selection(self): > self.openTestHtml(enabled=True) > # Currently, selection carets do not show on non-editable elements. Please delete this comment since selection carets supports non-editable now.

Update to address comment.

Latest try run still have some fails. Those fails because context menu is shown after calling long tap. Then context menu overlapp selection carets so that we cannot hit carets and tests fail. So I add a parameter to indicate visibility of context menu when calling long press.

latest try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=5c2141286e54

Comment on attachment 8520431 [details] [diff] [review] Part 5: Add a parameter to toggle context menu when calling long_press. There are something wrong in this patch. Clear review.

Comment on attachment 8520431 [details] [diff] [review] Part 5: Add a parameter to toggle context menu when calling long_press. Review of attachment 8520431 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/tests/marionette/test_selectioncarets.py @@ +47,5 @@ > > # Long press the caret position. Selection carets should appear, and the > # first word will be selected. On Windows, those spaces after the word > # will also be selected. > + self.actions.long_press(el, self._long_press_time, x, y,\ The arguments in the parentheses do not need a backslash at the end of the line. ::: testing/marionette/client/marionette/marionette.py @@ +380,5 @@ > self.action_chain.append(['wait', time_increment/1000]) > self.action_chain.append(['release']) > return self > > + def long_press(self, element, time_in_seconds, x=None, y=None, with_contextmenu=False): with_contextmenu should be default to True. @@ +388,5 @@ > :param element: The element to press. > :param time_in_seconds: Time in seconds to wait before releasing the press. > :param x: Optional, x-coordinate to tap, relative to the top-left > corner of the element. > :param y: Optional, y-coordinate to tap, relative to the top-left Please add document for "with_contextmenu" @@ +400,5 @@ > > ''' > element = element.id > self.action_chain.append(['press', element, x, y]) > + if with_contextmenu == True: if not with_contextmenu

Fix address comment.

latest try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=3b7446d8e1bb

Comment on attachment 8520440 [details] [diff] [review] Part 5: Add a parameter to toggle context menu when calling long_press v2. Review of attachment 8520440 [details] [diff] [review]: ----------------------------------------------------------------- r- just to move the call to gestures.py, looks good otherwise! ::: testing/marionette/client/marionette/marionette.py @@ +404,5 @@ > ''' > element = element.id > self.action_chain.append(['press', element, x, y]) > + if not with_contextmenu: > + self.action_chain.append(['moveByOffset', 0, 0]) I don't think this should be part of the marionette library. We want this library to be a direct interface to what the server side supports. Since the server doesn't support this, and the moveByOffset(0,0) is wholly dependent on the way gecko processes events, I don't want to add it here. That being said, it can be added to gestures.py without problem: https://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/gestures.py

Update address comment.

try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9f1ba923b07b

Comment on attachment 8523605 [details] [diff] [review] Part 5: Add a parameter to toggle context menu when calling long_press v3. Review of attachment 8523605 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/marionette/client/marionette/gestures.py @@ +73,5 @@ > +#element: The element to press. > +#time_in_seconds: Time in seconds to wait before releasing the press. > +#x: Optional, x-coordinate to tap, relative to the top-left corner of the element. > +#y: Optional, y-coordinate to tap, relative to the top-leftcorner of the element. > +def long_press_without_contextmenu(marionette_session, element, time_in_seconds, x=None, y=None): Nit: The triple quote docstring is the preferred way since it can be picked up by Python automatically.

(In reply to Ting-Yu Lin [:TYLin] (UTC+8) from comment #28) > Comment on attachment 8523605 [details] [diff] [review] > Part 5: Add a parameter to toggle context menu when calling long_press v3. > > Review of attachment 8523605 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: testing/marionette/client/marionette/gestures.py > @@ +73,5 @@ > > +#element: The element to press. > > +#time_in_seconds: Time in seconds to wait before releasing the press. > > +#x: Optional, x-coordinate to tap, relative to the top-left corner of the element. > > +#y: Optional, y-coordinate to tap, relative to the top-leftcorner of the element. > > +def long_press_without_contextmenu(marionette_session, element, time_in_seconds, x=None, y=None): > > Nit: The triple quote docstring is the preferred way since it can be picked > up by Python automatically. Hmm, I just followed code style in gestures.py.

Comment on attachment 8523605 [details] [diff] [review] Part 5: Add a parameter to toggle context menu when calling long_press v3. Review of attachment 8523605 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the fix! And yes, feel free to change the quotes style in gestures.py if you prefer them to be triple-quoted.

https://hg.mozilla.org/integration/mozilla-inbound/rev/54e551c1f2cd https://hg.mozilla.org/integration/mozilla-inbound/rev/c53a6181a4b2 https://hg.mozilla.org/integration/mozilla-inbound/rev/b1a0a7ea1a32 https://hg.mozilla.org/integration/mozilla-inbound/rev/db37104f6fbd https://hg.mozilla.org/integration/mozilla-inbound/rev/8c513f282a9e

https://hg.mozilla.org/mozilla-central/rev/54e551c1f2cd https://hg.mozilla.org/mozilla-central/rev/c53a6181a4b2 https://hg.mozilla.org/mozilla-central/rev/b1a0a7ea1a32 https://hg.mozilla.org/mozilla-central/rev/db37104f6fbd https://hg.mozilla.org/mozilla-central/rev/8c513f282a9e

Backout in below commit. https://hg.mozilla.org/integration/mozilla-inbound/rev/c0d50a3f1e96

rebased.

Should be ok for checking-in again.

https://hg.mozilla.org/integration/mozilla-inbound/rev/daee46827258 https://hg.mozilla.org/integration/mozilla-inbound/rev/be16d194d5df https://hg.mozilla.org/integration/mozilla-inbound/rev/86385291d8c8 https://hg.mozilla.org/integration/mozilla-inbound/rev/7cd8260bfc16 https://hg.mozilla.org/integration/mozilla-inbound/rev/0ad25135a27f

Something in the push from comment 36 made Gij(2) permafail (see the log link below). In the interest of getting the trees reopened sooner, I've backed out the entire push for now. I've also kicked off Try runs of each bug individually backed out and will re-land the innocent patches. Sorry for the hassle. https://hg.mozilla.org/integration/mozilla-inbound/rev/481af84aba2a https://treeherder.mozilla.org/ui/logviewer.html#?job_id=4455263&repo=mozilla-inbound

The Try push confirms that this was the cause of the permafail.

This Gij permafail is caused by app crash at DispatchSelectionStateChangeEvent. Bug 1067728 fix some logic here. And it has been landed. After rebased my patches and re-try, I don't encounter this permafail again. Here is my latest try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=11ebae746ac3 Ready for re-land again.

https://hg.mozilla.org/integration/b2g-inbound/rev/d83ffb51ea3c https://hg.mozilla.org/integration/b2g-inbound/rev/20fa997890ee https://hg.mozilla.org/integration/b2g-inbound/rev/dc46be880e79 https://hg.mozilla.org/integration/b2g-inbound/rev/c185027b1fb7 https://hg.mozilla.org/integration/b2g-inbound/rev/9fd5690408a4

https://hg.mozilla.org/mozilla-central/rev/d83ffb51ea3c https://hg.mozilla.org/mozilla-central/rev/20fa997890ee https://hg.mozilla.org/mozilla-central/rev/dc46be880e79 https://hg.mozilla.org/mozilla-central/rev/c185027b1fb7 https://hg.mozilla.org/mozilla-central/rev/9fd5690408a4