Closed Bug 1379808 Opened 7 years ago Closed 7 years ago

Intermittent browser_test_zoom_text.js | Wrong height of text between offsets (0, -1) for [DOM node id: p1, role: paragraph, address: [xpconnect wrapped (nsISupports, nsIAccessible, nsIAccessibleText)]] - Got 27, expected 28

Categories

(Core :: Disability Access APIs, defect, P2)

Product:
Core
Component:
Disability Access APIs
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox56 --- disabled
firefox57 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: surkov)

References

Details

(Keywords: intermittent-failure, Whiteboard: [stockwell disabled])

Attachments

(1 file)

patch
2.86 KB, patch
eeejay
: review+
Details | Diff | Splinter Review 
this is a recent failure that would be nice to fix if easy- yet not frequent enough for me to nag often.

Here is a recent log:
https://treeherder.mozilla.org/logviewer.html#?repo=pine&job_id=114878803

and the related failure text:
09:39:25     INFO - TEST-START | accessible/tests/browser/bounds/browser_test_zoom_text.js
09:39:25     INFO - GECKO(1666) | ++DOCSHELL 0x11d90f000 == 2 [pid = 1672] [id = {7ab36cc4-4bda-4446-9139-80b5448cde22}]
09:39:25     INFO - GECKO(1666) | ++DOMWINDOW == 3 (0x11d90f800) [pid = 1672] [serial = 3] [outer = 0x0]
09:39:25     INFO - GECKO(1666) | ++DOMWINDOW == 4 (0x11d915000) [pid = 1672] [serial = 4] [outer = 0x11d90f800]
09:39:25     INFO - TEST-INFO | started process screencapture
09:39:26     INFO - TEST-INFO | screencapture: exit 0
09:39:26     INFO - Buffered messages logged at 09:39:25
09:39:26     INFO - Entering test bound 
09:39:26     INFO - TEST-PASS | accessible/tests/browser/bounds/browser_test_zoom_text.js | Accessible document present. - 
09:39:26     INFO - TEST-PASS | accessible/tests/browser/bounds/browser_test_zoom_text.js | Wrong x coordinate of text between offsets (0, -1) for [DOM node id: p1, role: paragraph, address: 0x11a9d1140] - 
09:39:26     INFO - TEST-PASS | accessible/tests/browser/bounds/browser_test_zoom_text.js | Wrong y coordinate of text between offsets (0, -1) for [DOM node id: p1, role: paragraph, address: 0x11a9d1140] - 
09:39:26     INFO - TEST-PASS | accessible/tests/browser/bounds/browser_test_zoom_text.js | Wrong width of text between offsets (0, -1) for [DOM node id: p1, role: paragraph, address: 0x11a9d1140] - 
09:39:26     INFO - TEST-PASS | accessible/tests/browser/bounds/browser_test_zoom_text.js | Wrong height of text between offsets (0, -1) for [DOM node id: p1, role: paragraph, address: 0x11a9d1140] - 
09:39:26     INFO - TEST-PASS | accessible/tests/browser/bounds/browser_test_zoom_text.js | Wrong x coordinate of text between offsets (0, -1) for [DOM node id: p2, role: paragraph, address: 0x11a9d1260] - 
09:39:26     INFO - TEST-PASS | accessible/tests/browser/bounds/browser_test_zoom_text.js | Wrong y coordinate of text between offsets (0, -1) for [DOM node id: p2, role: paragraph, address: 0x11a9d1260] - 
09:39:26     INFO - TEST-PASS | accessible/tests/browser/bounds/browser_test_zoom_text.js | Wrong width of text between offsets (0, -1) for [DOM node id: p2, role: paragraph, address: 0x11a9d1260] - 
09:39:26     INFO - TEST-PASS | accessible/tests/browser/bounds/browser_test_zoom_text.js | Wrong height of text between offsets (0, -1) for [DOM node id: p2, role: paragraph, address: 0x11a9d1260] - 
09:39:26     INFO - TEST-PASS | accessible/tests/browser/bounds/browser_test_zoom_text.js | Wrong x coordinate of text between offsets (0, -1) for [DOM node id: p1, role: paragraph, address: 0x11a9d1140] - 
09:39:26     INFO - TEST-PASS | accessible/tests/browser/bounds/browser_test_zoom_text.js | Wrong y coordinate of text between offsets (0, -1) for [DOM node id: p1, role: paragraph, address: 0x11a9d1140] - 
09:39:26     INFO - TEST-PASS | accessible/tests/browser/bounds/browser_test_zoom_text.js | Wrong width of text between offsets (0, -1) for [DOM node id: p1, role: paragraph, address: 0x11a9d1140] - 
09:39:26     INFO - Buffered messages finished
09:39:26     INFO - TEST-UNEXPECTED-FAIL | accessible/tests/browser/bounds/browser_test_zoom_text.js | Wrong height of text between offsets (0, -1) for [DOM node id: p1, role: paragraph, address: 0x11a9d1140] - Got 27, expected 28
09:39:26     INFO - Stack trace:
09:39:26     INFO - chrome://mochikit/content/browser-test.js:test_is:967
09:39:26     INFO - chrome://mochitests/content/a11y/accessible/tests/mochitest/layout.js:testTextBounds:176
09:39:26     INFO - chrome://mochitests/content/browser/accessible/tests/browser/bounds/browser_test_zoom_text.js:testTextNode:15
09:39:26     INFO - chrome://mochitests/content/browser/accessible/tests/browser/bounds/browser_test_zoom_text.js:runTests:28
09:39:26     INFO - Leaving test bound 
09:39:26     INFO - GECKO(1666) | MEMORY STAT | vsize 4371MB | residentFast 389MB | heapAllocated 131MB
09:39:26     INFO - TEST-OK | accessible/tests/browser/bounds/browser_test_zoom_text.js | took 717ms
09:39:26     INFO - GECKO(1666) | ++DOCSHELL 0x11a415000 == 2 [pid = 1670] [id = {c70149bd-55de-cb4e-9c11-ddd742e12c9c}]

:surkov, do you think there is an easy fix to make this test more reliable?
Flags: needinfo?(surkov.alexander)
The test checks whether GetBounds and GetRangeExtents methods return same values. For some reason, a text node's height may not be equal a height of corresponding text range. Note, x/y/width are in sync. The difference in 1 pixel makes me think there's some layout's computation difference, but it's hard to say for sure with no proper expertise in the layout module.
Flags: needinfo?(surkov.alexander)
thanks for the info :surkov.  If this continues at the same rate of failures into next week, we should ask someone from the layout team to help out.
Whiteboard: [stockwell needswork]
giving it a P2 status as the failures ratio is high enough
Priority: -- → P2
Very frequent failures continue.

These failures started around the time the test was ported from mochitest to browser-chrome, in bug 1376852. I see no record of failures in the old mochitest. Should we consider a backout of bug 1376852?
Blocks: 1376852
Flags: needinfo?(eitan)
See Also: → 1391453
Pushed by jmaher@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ec5e6cc98d9b
disable browser_test_zoom_text.js. a=testonly
Keywords: leave-open
Whiteboard: [stockwell needswork] → [stockwell disabled]
apologies for disabling this, this was between a couple bugs and had a very high failure rate both individually and combined.
Sorry I am arriving at this late. This looks like it could be a float to int rounding issue. Maybe its worth while having it compare within one pixel accuracy?
Flags: needinfo?(eitan) → needinfo?(surkov.alexander)
(In reply to Eitan Isaacson [:eeejay] from comment #20)
> Sorry I am arriving at this late. This looks like it could be a float to int
> rounding issue. Maybe its worth while having it compare within one pixel
> accuracy?

I think 1 pixel error doesn't hurt anyone.
Flags: needinfo?(surkov.alexander)
Attached patch patch — — Splinter Review 
like this?
Assignee: nobody → surkov.alexander
Attachment #8906054 - Flags: review?(eitan)
Comment on attachment 8906054 [details] [diff] [review]
patch

Review of attachment 8906054 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good.

I would have made another "is"-like function: isWithin(aExpected, aGot, aWithin, aMsg). With aWithin being 1.

::: accessible/tests/mochitest/layout.js
@@ +161,5 @@
> +    ok(true,
> +       `Correct y of text between offsets (${aStartOffset}, ${aEndOffset}) ` +
> +       `for ${prettyName(aID)} - Got ${yObj.value}`);
> +  }
> +  else {

Do these files get linted? If so, I think the `else {` needs to be on the same line as the previous block end (`}`).
Attachment #8906054 - Flags: review?(eitan) → review+
(In reply to Eitan Isaacson [:eeejay] from comment #23)

> I would have made another "is"-like function: isWithin(aExpected, aGot,
> aWithin, aMsg). With aWithin being 1.

yeah, I was thinking about this but was too lazy implement it, I'll do it to not get back to it again :) 

> ::: accessible/tests/mochitest/layout.js

> Do these files get linted? If so, I think the `else {` needs to be on the
> same line as the previous block end (`}`).

not sure, but I'll do the change
https://hg.mozilla.org/integration/mozilla-inbound/rev/435bc55b9c14052b4206dd5734de0049fe70eb53
Bug 1379808 - Intermittent browser_test_zoom_text.js failure, wrong height and y, r=eeejay
Backed out for failing browser-chrome's accessible/tests/browser/bounds/browser_test_zoom_text.js on Windows:

https://hg.mozilla.org/integration/mozilla-inbound/rev/1852c4dfd856e5adcf98d00da8204c7f2ee250ac

Push which ran failing tests: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=bc1f526a6152eb8a810c78041678b249c0906314&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=130122988&repo=mozilla-inbound

17:11:18     INFO -  5 INFO TEST-START | accessible/tests/browser/bounds/browser_test_zoom_text.js
17:11:18     INFO -  TEST-INFO | started process screenshot
17:11:18     INFO -  TEST-INFO | screenshot: exit 0
17:11:18     INFO -  Buffered messages logged at 17:11:18
17:11:18     INFO -  6 INFO Entering test bound
17:11:18     INFO -  7 INFO Console message: [JavaScript Warning: "Use of nsIFile in content process is deprecated." {file: "resource://gre/modules/FileUtils.jsm" line: 174}]
17:11:18     INFO -  Buffered messages finished
17:11:18    ERROR -  8 INFO TEST-UNEXPECTED-FAIL | accessible/tests/browser/bounds/browser_test_zoom_text.js | Uncaught exception - [Exception... "Component returned failure code: 0x80004001 (NS_ERROR_NOT_IMPLEMENTED) [nsIAccessibleText.getRangeExtents]"  nsresult: "0x80004001 (NS_ERROR_NOT_IMPLEMENTED)"  location: "JS frame :: chrome://mochitests/content/a11y/accessible/tests/mochitest/layout.js :: testTextBounds :: line 151"  data: no]
17:11:18     INFO -  Stack trace:
17:11:18     INFO -      testTextBounds@chrome://mochitests/content/a11y/accessible/tests/mochitest/layout.js:151:3
17:11:18     INFO -      testTextNode@chrome://mochitests/content/browser/accessible/tests/browser/bounds/browser_test_zoom_text.js:15:5
17:11:18     INFO -      runTests@chrome://mochitests/content/browser/accessible/tests/browser/bounds/browser_test_zoom_text.js:21:3
17:11:18     INFO -      async*addAccessibleTask/</<@chrome://mochitests/content/browser/accessible/tests/browser/shared-head.js:286:13
17:11:18     INFO -      async*withNewTab@resource://testing-common/BrowserTestUtils.jsm:105:24
17:11:18     INFO -      async*addAccessibleTask/<@chrome://mochitests/content/browser/accessible/tests/browser/shared-head.js:262:11
17:11:18     INFO -      Async*Tester_execTest/<@chrome://mochikit/content/browser-test.js:803:21
17:11:18     INFO -      Tester_execTest@chrome://mochikit/content/browser-test.js:794:9
17:11:18     INFO -      Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:694:7
17:11:18     INFO -      SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59
Flags: needinfo?(surkov.alexander)
Eitan, do you know if anything has been changed recently, so re-enabled test started failing at nsIAccessibleText.getRangeExtents (not implemented)? Is it about the mode the test is running under?
Flags: needinfo?(surkov.alexander) → needinfo?(eitan)
Yeah, we don't proxy that method yet in windows. So this needs to be skipped there.

See the above patch that disabled the test:
https://hg.mozilla.org/mozilla-central/rev/ec5e6cc98d9b
Flags: needinfo?(eitan)
(In reply to Eitan Isaacson [:eeejay] from comment #29)
> Yeah, we don't proxy that method yet in windows. So this needs to be skipped
> there.

how to enable the test for in-content testing only?

> See the above patch that disabled the test:
> https://hg.mozilla.org/mozilla-central/rev/ec5e6cc98d9b

it was disabled for intermittent failures, right?
(In reply to alexander :surkov from comment #30)
> (In reply to Eitan Isaacson [:eeejay] from comment #29)
> > Yeah, we don't proxy that method yet in windows. So this needs to be skipped
> > there.
> 
> how to enable the test for in-content testing only?

skip-if = e10s && (os == 'win')
https://hg.mozilla.org/integration/mozilla-inbound/rev/775a86f9a091f1b86225ea866f04b166968bced9
Bug 1379808 - Intermittent browser_test_zoom_text.js failure, wrong height and y, r=eeejay
Status: NEW → RESOLVED
Closed: 7 years ago
Keywords: leave-open
Resolution: --- → FIXED
status-firefox57: disabled → fixed
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: