1357998 - Permaorange in test_bug430392.html and friends when Gecko 55 merges to beta on 2017-06-12

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect, P1)

Description

[Phil Ringnalda (:philor)]

Not sure offhand what part of it depends on something that's only enabled for NIGHTLY_BUILD or is disabled on RELEASE_OR_BETA, but trunk-as-beta try simulation (which you should be able to reproduce locally by just changing /config/milestone.txt from 55a1 to 55) says that there will be a bunch of return-related editor mochitest failures, https://treeherder.mozilla.org/logviewer.html#?job_id=92882737&repo=try

[Tracking Requested - why for this release]:

Merge bustage, closed tree, delayed b1, relman raging.

Comment 1

[Phil Ringnalda (:philor)]

And https://treeherder.mozilla.org/logviewer.html#?job_id=92882717&repo=try is probably the same problem, in mochitest-chrome.

Comment 2

[Phil Ringnalda (:philor)]

Probably my last one, test_bug551704.html in mochitest-clipboard, https://treeherder.mozilla.org/logviewer.html#?job_id=92882623&repo=try

Comment 3

[Phil Ringnalda (:philor)]

Where by last I meant next-to-last, because there's also web-platform-tests, https://treeherder.mozilla.org/logviewer.html#?job_id=92882685&repo=try

Comment 4

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Whoops, sorry.  This didn't occur to me when doing bug 1357482.  All these tests need the pref editor.use_div_for_default_newlines to be set to true.  Is correct procedure here to edit the appropriate mochitest.ini and so on to set the pref to true unconditionally on central?  (I know wpt has syntax for this, probably mochitest does too?)  I can try to write a patch if it would be easier for you than doing it yourself.

Comment 5

[Marcia Knous [:marcia]]

Tracking 55+ for this permaorange, especially to avoid delays in B1.

Comment 6

[Phil Ringnalda (:philor)]

I'd rather have you do it, with awareness of the reason I don't want to do it:

In the case of new tests you wrote for bug 1357482 which only test its changes, sure, set the pref, but in the case of existing tests which you changed to reflect a new world, I don't think setting the pref is the right thing to do, because that means we would be testing what we don't ship, and not testing what we do ship, so it seems to me that tests like test_bug551704.html need to make a runtime determination of what they should expect based on the pref value, and then test for that.

Comment 7

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

For the mochitests I can do something like that, but for wpt it looks like we have no option but to force the pref.  We can't alter the tests to have Gecko-specific logic, and jgraham tells me our expected failures can't depend on the current pref values.

In most cases, the mochitests would be improved by testing all values of defaultParagraphSeparator anyway and not just the default.  In other cases they should just force the default.  I'll work on that now.

Comment 8

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Bug 1357998 - Make editing tests independent of default defaultParagraphSeparator;

Now that this is a pref that is different in different versions, tests
have to work no matter what the pref's value is.  For tests that
actually tested line-breaking-related behavior, I made them test all
three separator values.  For tests that tested something else and only
incidentally depend on the default paragraph separator, I set
defaultParagraphSeparator to "div".

Review commit: https://reviewboard.mozilla.org/r/132152/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/132152/

Comment 9

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

I did a second try run (in addition to the one on MozReview) with the pref forced to false as well:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6b204848ccfe1c9909b732b368006025ad592bc5

Comment 10

[Phil Ringnalda (:philor)]

Sounds good: the draft of my comment that I wrote in my head in the shower actually did say "just set the pref for wpt, that's the best we can do, but for the rest..."

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8860126 [details]
Bug 1357998 - Make editing tests independent of default defaultParagraphSeparator;

https://reviewboard.mozilla.org/r/132152/#review135160

Sorry, I should've realized this point.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0f6499e3b0fe
Make editing tests independent of default defaultParagraphSeparator; r=masayuki

Comment 13

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/0f6499e3b0fe