Closed
Bug 932296
Opened 11 years ago
Closed 11 years ago
Fix and re-enable layout/generic/test/test_bug448987.html
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla28
People
(Reporter: emorley, Assigned: MatsPalmgren_bugz)
References
Details
Attachments
(1 file, 1 obsolete file)
4.36 KB,
patch
|
martijn.martijn
:
review+
|
Details | Diff | Splinter Review |
layout/generic/test/test_bug448987.html is now failing with "Too often tried to focus image map, giving up", eg:
https://tbpl.mozilla.org/php/getParsedLog.php?id=29835269&tree=Mozilla-Inbound
{
06:29:29 INFO - 211 INFO TEST-INFO | /tests/layout/generic/test/test_bug448987.html | must wait for load
06:29:29 INFO - ++DOMWINDOW == 52 (0x11dda9818) [serial = 85] [outer = 0x11162f5f8]
06:29:30 INFO - --DOCSHELL 0x11d4e46d0 == 11 [id = 11]
06:29:30 INFO - --DOMWINDOW == 51 (0x11ed63548) [serial = 16] [outer = 0x0] [url = http://mochi.test:8888/tests/layout/generic/test/test_bug240933.html]
06:29:30 INFO - --DOMWINDOW == 50 (0x1116f8668) [serial = 26] [outer = 0x0] [url = http://mochi.test:8888/tests/layout/generic/test/test_bug288789.html]
06:29:30 INFO - --DOMWINDOW == 49 (0x1288abab8) [serial = 56] [outer = 0x11dd11b48] [url = data:text/html;charset=utf-8,%3Chtml%3E%0A%3Chead%3E%0A%3Cstyle%3E%0Adiv%3A%3Afirst-letter%20%7B%20%7D%0Aspan%3A%3Aafter%20%7B%20content%3A%22before%20text%22%3B%20%7D%0A%3C/style%3E%0A%3C/head%3E%0A%3Cbody%3E%0A%3Cdiv%20style%3D%22position%3A%20fixed%3B%20direction%3A%20rtl%3B%22%3E%0A%20%20%3Cspan%20style%3D%22%20direction%3A%20ltr%3B%20unicode-bidi%3A%20bidi-override%3B%20font-size%3A%2070px%3B%20%22%3E%0A%20%20%20%20%3Cspan%20style%3D%22display%3A%20table%3B%20position%3A%20fixed%3B%22%3E%3C/span%3E%0A%20%20%3C/span%3E%0A%3C/div%3E%0A%3Cscript%3E%0Afunction%20doe%28i%29%7B%0Adocument.documentElement.setAttribute%28%27style%27%2C%20%27%27%29%3B%0A%7D%0AsetTimeout%28doe%2C100%29%3B%0A%3C/script%3E%0A%3C/body%3E%0A%3C/html%3E]
06:29:30 INFO - --DOMWINDOW == 48 (0x11dd11b48) [serial = 55] [outer = 0x0] [url = data:text/html;charset=utf-8,%3Chtml%3E%0A%3Chead%3E%0A%3Cstyle%3E%0Adiv%3A%3Afirst-letter%20%7B%20%7D%0Aspan%3A%3Aafter%20%7B%20content%3A%22before%20text%22%3B%20%7D%0A%3C/style%3E%0A%3C/head%3E%0A%3Cbody%3E%0A%3Cdiv%20style%3D%22position%3A%20fixed%3B%20direction%3A%20rtl%3B%22%3E%0A%20%20%3Cspan%20style%3D%22%20direction%3A%20ltr%3B%20unicode-bidi%3A%20bidi-override%3B%20font-size%3A%2070px%3B%20%22%3E%0A%20%20%20%20%3Cspan%20style%3D%22display%3A%20table%3B%20position%3A%20fixed%3B%22%3E%3C/span%3E%0A%20%20%3C/span%3E%0A%3C/div%3E%0A%3Cscript%3E%0Afunction%20doe%28i%29%7B%0Adocument.documentElement.setAttribute%28%27style%27%2C%20%27%27%29%3B%0A%7D%0AsetTimeout%28doe%2C100%29%3B%0A%3C/script%3E%0A%3C/body%3E%0A%3C/html%3E]
06:29:31 INFO - 212 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/generic/test/test_bug448987.html | Too often tried to focus image map, giving up
06:29:31 INFO - JavaScript error: http://mochi.test:8888/tests/layout/generic/test/file_bug448987.html, line 36: parent.Simpletest is undefined
06:29:31 INFO - 213 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/generic/test/test_bug448987.html | Too often tried to focus image map, giving up
06:29:31 INFO - JavaScript error: http://mochi.test:8888/tests/layout/generic/test/file_bug448987.html, line 36: parent.Simpletest is undefined
06:29:31 INFO - 214 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/generic/test/test_bug448987.html | Too often tried to focus image map, giving up
06:29:31 INFO - JavaScript error: http://mochi.test:8888/tests/layout/generic/test/file_bug448987.html, line 36: parent.Simpletest is undefined
06:29:31 INFO - 215 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/generic/test/test_bug448987.html | Too often tried to focus image map, giving up
06:29:31 INFO - JavaScript error: http://mochi.test:8888/tests/layout/generic/test/file_bug448987.html, line 36: parent.Simpletest is undefined
06:29:31 INFO - 216 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/generic/test/test_bug448987.html | Too often tried to focus image map, giving up
06:29:31 INFO - JavaScript error: http://mochi.test:8888/tests/layout/generic/test/file_bug448987.html, line 36: parent.Simpletest is undefined
06:29:31 INFO - 217 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/generic/test/test_bug448987.html | Too often tried to focus image map, giving up
06:29:31 INFO - JavaScript error: http://mochi.test:8888/tests/layout/generic/test/file_bug448987.html, line 36: parent.Simpletest is undefined
06:29:31 INFO - 218 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/generic/test/test_bug448987.html | Too often tried to focus image map, giving up
06:29:31 INFO - JavaScript error: http://mochi.test:8888/tests/layout/generic/test/file_bug448987.html, line 36: parent.Simpletest is undefined
}
...after this landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/66d14f6329ca
The testing is too sensitive to timing changes and so has been disabled until fixed.
Reporter | ||
Comment 1•11 years ago
|
||
Test disabled in:
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/5c3c2060a723
Also in the error case, there is a typo:
http://mxr.mozilla.org/mozilla-central/source/layout/generic/test/file_bug448987.html?force=1#36
34 if (counter > 10) {
35 parent.ok(false, "Too often tried to focus image map, giving up");
36 parent.Simpletest.finish();
37 return;
38 }
Please s/Simpletest/SimpleTest/ as well as whatever other fixes required, before re-enabling :-)
Comment 2•11 years ago
|
||
The timers are necessary until bug 922524 is fixed and I don't know how to fix that :(
Indeed, really bad that this started failing on MacOS X after some mochitests were disabled (bug 921635) on Mac.
Comment 3•11 years ago
|
||
Also, did it fail across all MacOS X machines?
It may have something to do with MacOS X accessibility pref of skipping focus of things other than links and form controls. Although, I wouldn't know how this would be giving errors only now, then.
Reporter | ||
Comment 4•11 years ago
|
||
Yeah it failed on all OS X versions:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=66d14f6329ca&jobname=osx
Comment 5•11 years ago
|
||
I should add accessibility.tabfocus pref to 7 for the Mac:
http://mxr.mozilla.org/mozilla-central/source/content/events/test/test_bug238987.html?force=1#34
That might be the reason it is failing consistently now for the Mac. No idea why they would pass beforehand, though.
Reporter | ||
Comment 6•11 years ago
|
||
(In reply to Martijn W[:mwargers] (QA) [PTO 21-27/10] from comment #5)
> That might be the reason it is failing consistently now for the Mac. No idea
> why they would pass beforehand, though.
This test only started only failing when https://hg.mozilla.org/integration/mozilla-inbound/rev/66d14f6329ca landed, which only affected OS X.
Assignee | ||
Comment 7•11 years ago
|
||
Maybe one of the tests in that changeset changed accessibility.tabfocus without resetting it?
Assignee | ||
Comment 8•11 years ago
|
||
This appears to work:
https://tbpl.mozilla.org/?tree=Try&rev=7e04fb452d13
Attachment #824586 -
Flags: review?(emorley)
Comment 9•11 years ago
|
||
Comment on attachment 824586 [details] [diff] [review]
fix
Thanks, Mats, this looks good, I was also looking on fixing this.
You need to add
clearInterval(timer);
after the
+ parent.SimpleTest.finish();
Also, the same fix is needed for file_bug448987_ref.html
Comment 10•11 years ago
|
||
Possible and likely explanation of what happened:
With the removal of the dom-level tests, the chunking got changed.
Now test_bug411236.html isn't run before test_bug448987.html anymore, while it used to. And test_bug411236.html contains this accessibility.tabfocus=7 pref code.
Unsetting the pref to what it was doesn't work correctly, and the accessibility.tabfocus=7 behavior would continue.
I tested this with:
SpecialPowers.setIntPref("accessibility.tabfocus", 7);
SpecialPowers.clearUserPref("accessibility.tabfocus");
After the clearUserPref, the test would still pass, meaning the old accessibility.tabfocus=7 behavior continues.
Perhaps bug 343600 is related to this?
In any case, here are some mxr links that seems the cause of this problem:
Inside nsFocusManager::DetermineElementToMoveFocus
http://mxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp#2340
nsIContent::sTabFocusModel is being set from LookAndFeel::GetInt
Here the 'listen for pref changes' code is called (Registercallback):
http://mxr.mozilla.org/mozilla-central/source/widget/xpwidgets/nsXPLookAndFeel.cpp#436
And apparently that doesn't handle removal of preferences, which would be related to bug 343600 then.
Assignee | ||
Comment 11•11 years ago
|
||
(In reply to Martijn W[:mwargers] (QA) [PTO 21-27/10] from comment #9)
Good catch, thanks.
https://tbpl.mozilla.org/?tree=Try&rev=30b74441bb20
Attachment #824586 -
Attachment is obsolete: true
Attachment #824586 -
Flags: review?(emorley)
Attachment #824619 -
Flags: review?(martijn.martijn)
Assignee | ||
Comment 12•11 years ago
|
||
(In reply to Martijn W[:mwargers] (QA) [PTO 21-27/10] from comment #10)
> Unsetting the pref to what it was doesn't work correctly
Please file a bug about this.
Clearly DetermineElementToMoveFocus needs to take the pref into account
for example. The OSX system pref should only be used when accessibility.tabfocus
is unset according to the documentation:
https://developer.mozilla.org/en-US/docs/Mozilla/Preferences/Preference_reference/accessibility.tabfocus
Comment 13•11 years ago
|
||
Comment on attachment 824619 [details] [diff] [review]
fix, v2
Looks good, thanks!
Attachment #824619 -
Flags: review?(martijn.martijn) → review+
Comment 14•11 years ago
|
||
(In reply to Mats Palmgren (:mats) from comment #12)
> (In reply to Martijn W[:mwargers] (QA) [PTO 21-27/10] from comment #10)
> > Unsetting the pref to what it was doesn't work correctly
>
> Please file a bug about this.
Ok, filed bug 932814.
Assignee | ||
Updated•11 years ago
|
Reporter | ||
Comment 15•11 years ago
|
||
Thank you for figuring this out! :-)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → matspal
Status: NEW → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in
before you can comment on or make changes to this bug.
Description
•