Closed
Bug 1383512
Opened 7 years ago
Closed 7 years ago
Intermittent dom/base/test/test_user_select.html | test1: selected text - got "aaaaaccccccc", expected "aaaaaacc"
Categories
(Core :: DOM: Selection, defect)
Core
DOM: Selection
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: intermittent-bug-filer, Assigned: mantaroh)
Details
(Keywords: intermittent-failure, Whiteboard: [stockwell fixed:race])
Attachments
(2 files, 1 obsolete file)
Filed by: archaeopteryx [at] coole-files.de https://treeherder.mozilla.org/logviewer.html#?job_id=116689768&repo=mozilla-inbound https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-inbound-win32/1500762019/mozilla-inbound_win7_vm_test-mochitest-e10s-1-bm140-tests1-windows-build309.txt.gz 17:35:22 INFO - TEST-START | dom/base/test/test_user_select.html 17:35:22 INFO - TEST-INFO | started process screenshot 17:35:22 INFO - TEST-INFO | screenshot: exit 0 17:35:22 INFO - TEST-UNEXPECTED-FAIL | dom/base/test/test_user_select.html | test1: selected text - got "aaaaaccccccc", expected "aaaaaacc" 17:35:22 INFO - SimpleTest.is@SimpleTest/SimpleTest.js:312:5 17:35:22 INFO - checkText@dom/base/test/test_user_select.html:107:5 17:35:22 INFO - test@dom/base/test/test_user_select.html:153:3 17:35:22 INFO - setTimeout handler*SimpleTest_setTimeoutShim@SimpleTest/SimpleTest.js:676:12 17:35:22 INFO - window.onload@dom/base/test/test_user_select.html:336:30 17:35:22 INFO - EventHandlerNonNull*@dom/base/test/test_user_select.html:336:1
Comment hidden (Intermittent Failures Robot) |
Updated•7 years ago
|
Priority: -- → P5
Comment hidden (Intermittent Failures Robot) |
Comment 3•7 years ago
|
||
Hi Jet, is this something your team could take a look?
Component: DOM → Selection
Flags: needinfo?(bugs)
Comment hidden (Intermittent Failures Robot) |
Comment 5•7 years ago
|
||
I think this test is racy because it depends on the Ahem font being loaded and used for the text fields, which are then selected using device pixel offsets. Switching it from window.onload to body.onload like this test, should help: http://searchfox.org/mozilla-central/source/dom/base/test/test_caretPositionFromPoint.html#114
Comment 6•7 years ago
|
||
Mantaroh, would you mind looking into fixing this test?
Flags: needinfo?(bugs) → needinfo?(mantaroh)
Assignee | ||
Comment 7•7 years ago
|
||
(In reply to Brian Birtles (:birtles, away 31 July~7 Aug) from comment #6) > Mantaroh, would you mind looking into fixing this test? OK. I will do that.
Assignee: nobody → mantaroh
Status: NEW → ASSIGNED
Flags: needinfo?(mantaroh)
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment 10•7 years ago
|
||
:mantaroh - Are you making progress here? Unless a fix is coming soon, I'd like to disable the test while you work on it.
Flags: needinfo?(mantaroh)
Assignee | ||
Comment 11•7 years ago
|
||
I think that this phenomenon is timing issue. I couldn't reproduce this phenomenon on my laptop(win7VM / PGO build). But I confirmed that we can't reproduce this phenomenon by using following waiting code on try[1]: * wait window.load() * wait FontFace.load() for Ahem font. * wait requestAnimationFrame() for styling. [1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=391e2633bb9bf2d68cb78d9ff4c09803e886d894 (In reply to Geoff Brown [:gbrown] from comment #10) > :mantaroh - Are you making progress here? Unless a fix is coming soon, I'd > like to disable the test while you work on it. OK. I'll disable this test on windows pgo and opt until clarify this reason. try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5eb04e77e4ba6611bda1f5a0490736e3f8960000
Flags: needinfo?(mantaroh)
Comment hidden (mozreview-request) |
Comment 13•7 years ago
|
||
mozreview-review |
Comment on attachment 8895227 [details] Bug 1383512 - Skip test_user_select.html on windows of opt and pgo. https://reviewboard.mozilla.org/r/166372/#review171700 Thanks!
Attachment #8895227 -
Flags: review?(gbrown) → review+
Comment 14•7 years ago
|
||
Pushed by gbrown@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3a17b61bc3e8 Skip test_user_select.html on windows of opt and pgo. r=gbrown
Assignee | ||
Comment 15•7 years ago
|
||
(In reply to Pulsebot from comment #14) > Pushed by gbrown@mozilla.com: > https://hg.mozilla.org/integration/autoland/rev/3a17b61bc3e8 > Skip test_user_select.html on windows of opt and pgo. r=gbrown Thanks Geoff for pushing it. Added leave-open keyword. I'll continue to investigate this phenomenon.
Keywords: leave-open
Comment 16•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3a17b61bc3e8
Updated•7 years ago
|
Whiteboard: [stockwell needswork:DOM] → [stockwell disabled]
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 18•7 years ago
|
||
(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #15) > (In reply to Pulsebot from comment #14) > > Pushed by gbrown@mozilla.com: > > https://hg.mozilla.org/integration/autoland/rev/3a17b61bc3e8 > > Skip test_user_select.html on windows of opt and pgo. r=gbrown > > Thanks Geoff for pushing it. > > Added leave-open keyword. I'll continue to investigate this phenomenon. I discussed with Brian. Maybe, We will need to wait to the following event: * window.load : To load all resouces. * FontFaceSet.ready: To load all fonts. * requestAnimationFrame : To style. Try : https://treeherder.mozilla.org/#/jobs?repo=try&revision=c8081278c422ffd5dfd2264f1cbd0e6b1c2aca31
Assignee | ||
Comment 19•7 years ago
|
||
Attachment #8900997 -
Flags: review?(mats)
Attachment #8900997 -
Flags: review?(bbirtles)
Assignee | ||
Comment 20•7 years ago
|
||
Comment on attachment 8900997 [details] [diff] [review] Bug 1383512 - Add waiting code for FontFaceSet.ready and requestAnimationFrame into test_user_select.html ># HG changeset patch ># Parent f0abd25e1f4acced652d180c34b7c9eda638deb1 >Bug 1383512 - Add waiting code until loading FontSet and finishing styling to test_user_select.html. r?mats,birtles > >In some environment, test will start before applying font face. >To ensure that content apply font, this patch changes to wait for starting test >until loading FontSet and finishing styling. > >MozReview-Commit-ID: 25AqGDwQfBt > >diff --git a/dom/base/test/mochitest.ini b/dom/base/test/mochitest.ini >--- a/dom/base/test/mochitest.ini >+++ b/dom/base/test/mochitest.ini >@@ -785,7 +785,7 @@ skip-if = debug == true && toolkit == 'a > support-files = file_title.xul > [test_treewalker_nextsibling.xml] > [test_user_select.html] >-skip-if = (toolkit == 'android') || (os == 'win' && !debug) # Windows is timing dependent(bug 1383512) >+skip-if = toolkit == 'android' > [test_viewport_scroll.html] > [test_viewsource_forbidden_in_object.html] > [test_w3element_traversal.html] >diff --git a/dom/base/test/test_user_select.html b/dom/base/test/test_user_select.html >--- a/dom/base/test/test_user_select.html >+++ b/dom/base/test/test_user_select.html >@@ -333,7 +333,18 @@ function test() > clear(); > SimpleTest.finish(); > } >-window.onload = function() { setTimeout(test, 0); }; >+ >+// Wait to window.onload() / FontFaceSet.ready() / requestAnimationFrame >+// in order to ensure that load font resources and style it. >+window.onload = function() { >+ document.fonts.ready.then(function() { >+ requestAnimationFrame(function() { >+ test(); >+ }); >+ }); >+}; >+ >+ > SimpleTest.waitForExplicitFinish(); > </script> > </pre>
Attachment #8900997 -
Attachment is patch: true
Comment 21•7 years ago
|
||
Comment on attachment 8900997 [details] [diff] [review] Bug 1383512 - Add waiting code for FontFaceSet.ready and requestAnimationFrame into test_user_select.html Review of attachment 8900997 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the following comments addressed. (Not sure you need two reviewers for this.) ::: dom/base/test/test_user_select.html @@ +334,5 @@ > SimpleTest.finish(); > } > + > +// Wait to window.onload() / FontFaceSet.ready() / requestAnimationFrame > +// in order to ensure that load font resources and style it. // These tests depend on the Ahem font being loaded and rendered so wait for // fonts to load, then wait a frame for them to be rendered too. @@ +335,5 @@ > } > + > +// Wait to window.onload() / FontFaceSet.ready() / requestAnimationFrame > +// in order to ensure that load font resources and style it. > +window.onload = function() { I'm curious, do we actually need to wait for onload here or is waiting for fonts enough? @@ +339,5 @@ > +window.onload = function() { > + document.fonts.ready.then(function() { > + requestAnimationFrame(function() { > + test(); > + }); Replace: requestAnimationFrame(function() { test(); }); with: requestAnimationFrame(test); @@ +343,5 @@ > + }); > + }); > +}; > + > + Nit: No need to two blank lines here.
Attachment #8900997 -
Flags: review?(bbirtles) → review+
Assignee | ||
Comment 22•7 years ago
|
||
Thank you for reviewing it. I think that this test need to wait for window.onload in order to prevent catch the callback which unrelated rendering. If removed this code, test was fail. Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9e89cdd99447a94020a7a09b642e9c766df05f74&group_state=expanded I fixed this test.
Attachment #8900997 -
Attachment is obsolete: true
Attachment #8900997 -
Flags: review?(mats)
Attachment #8901024 -
Flags: review+
Assignee | ||
Comment 23•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b5e302cfe5c4d3c3140f0be2603dcca36954b14f
Comment 24•7 years ago
|
||
Pushed by mantaroh@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/5e88f7a90eea Add waiting code until loading FontSet and finishing styling to test_user_select.html. r=birtles
Comment 25•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5e88f7a90eea
Updated•7 years ago
|
Whiteboard: [stockwell disabled] → [stockwell fixed:race]
Comment 26•7 years ago
|
||
Mantaroh-san, I think you can drop the leave-open flag now and close this bug, right?
Flags: needinfo?(mantaroh)
Assignee | ||
Comment 27•7 years ago
|
||
(In reply to Brian Birtles (:birtles) from comment #26) > Mantaroh-san, I think you can drop the leave-open flag now and close this > bug, right? Oh, Sorry. I forgot to clear this flag. Thanks!
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: needinfo?(mantaroh)
Keywords: leave-open
Resolution: --- → FIXED
Version: unspecified → Trunk
Updated•7 years ago
|
status-firefox57:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•