Closed Bug 560888 Opened 12 years ago Closed 12 years ago

accessible/tests/mochitest/attributes/test_text.html is failing for Fedora unit tests

Categories

(Core :: Disability Access APIs, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: armenzg, Assigned: davidb)

References

Details

Attachments

(1 file, 2 obsolete files)

From bug 554934#c13:
(In reply to comment #13)
> (In reply to comment #5)
> > Created an attachment (id=434879) [details] [details]
> > mochitest-a11y -
> > chrome://mochikit/content/a11y/accessible/attributes/test_text.html
> 
> Ok, here's a clue as to what's happening with reftest.  If you go look at this
> test:
> http://mxr.mozilla.org/mozilla-central/source/accessible/tests/mochitest/attributes/test_text.html?force=1
> you will see that this thing really doesn't do a good job of setting a default
> known state before tickling a bunch of font properties that all will be
> affected by the system's default settings.  
> 
> Digging some ancient (and possibly incorrect) information out of my head, I
> recall there is a default CSS used for each OS in the browser and in lieu of
> any user CSS direction those values are used.  They are used to set things like
> default font, font size, font family etc.  So, if those default settings aren't
> what they are on centos then this test could do something unexpected.  This one
> is really a fault of the test because it doesn't appear (in my quick scan) to
> set all the defaults to the known states that it is expecting.  If we were to
> change those default CSS values then we'd also break this test, I think.
> 
> And that might be related to what's going on with those reftest font-face
> tests.

1595 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/a11y/accessible/attributes/test_text.html | Attribute 'font-size' has wrong value. Getting default text attributes for area8 - got "8pt", expected "10pt"
1604 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/a11y/accessible/attributes/test_text.html | Attribute 'font-size' has wrong value for area8 at offset 0 - got "8pt", expected "10pt"
1614 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/a11y/accessible/attributes/test_text.html | Attribute 'font-size' has wrong value for area8 at offset 11 - got "8pt", expected "10pt"
1624 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/a11y/accessible/attributes/test_text.html | Attribute 'font-size' has wrong value for area8 at offset 17 - got "8pt", expected "10pt"
1634 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/a11y/accessible/attributes/test_text.html | Attribute 'font-size' has wrong value for area8 at offset 18 - got "8pt", expected "10pt"

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1271864813.1271866635.10140.gz
OK sounds like our tests need to check the desktop settings somehow. Right now they are hard coded, and obviously fragile. Do we have precedent for checking such things from mochitests?
No longer blocks: fedora-oranges
I think mostly we use systemfont as default.  CCing some people who might have an idea what needs to be done here.
Would it be testing what is intended if the font-size were set on the input?
Or perhaps getComputedStyle could be used on the input?
(In reply to comment #3)
> Would it be testing what is intended if the font-size were set on the input?

You would think. Doesn't seem to be work for input. I'm not sure why.

> Or perhaps getComputedStyle could be used on the input?

We have to expose the font size in pt units so in C++ we do some unpleasant contortions. getComputedStyle seems to return px and we're not sure how to do the same contortions in the test (in js). For this reason I think our testing of font size in test_text.html is inherently fragile.

I can make the test more forgiving for inputs.
Assignee: nobody → bolterbugz
Attachment #441223 - Flags: review?(surkov.alexander)
Attachment #441223 - Flags: review?(marco.zehe)
Attachment #441223 - Flags: review?(surkov.alexander)
Attachment #441223 - Flags: review?(marco.zehe)
Attached patch patch 2 (sniff fedora) (obsolete) — Splinter Review
Armen can you give this a whirl?
Attachment #441223 - Attachment is obsolete: true
Attachment #441230 - Flags: review?(surkov.alexander)
Attachment #441230 - Flags: feedback?(armenzg)
Attachment #441230 - Flags: review?(surkov.alexander) → review+
Comment on attachment 441230 [details] [diff] [review]
patch 2 (sniff fedora)

The sniffing doesn't seem to be working and we still get the same test failures.
Attachment #441230 - Flags: feedback?(armenzg) → feedback-
Ah right, that sniffer probably only works on Fedora packaged builds. Hmm.
(In reply to comment #8)
> Ah right, that sniffer probably only works on Fedora packaged builds. Hmm.

Nope it should work.
Heh, nope again. Just confirmed my comment 8. I'll have to go with something like the earlier approach.
Comment on attachment 441223 [details] [diff] [review]
patch - skip font size check for input

Marco, this allows us to bypass testing font size for input elements. We can't sniff for linux distro in our builds.
Attachment #441223 - Attachment is obsolete: false
Attachment #441223 - Flags: review?(marco.zehe)
Attachment #441230 - Attachment is obsolete: true
Comment on attachment 441223 [details] [diff] [review]
patch - skip font size check for input

I'd still like a better way to test this in the long run, esp when it comes to rich-editing in Google Docs where the font size is important. So we should add another bug to rework this for textboxes or the like. But r=me for this for now so we can get Fedora going.
Attachment #441223 - Flags: review?(marco.zehe) → review+
(In reply to comment #12)
> (From update of attachment 441223 [details] [diff] [review])
> I'd still like a better way to test this in the long run, esp when it comes to
> rich-editing in Google Docs where the font size is important. So we should add
> another bug to rework this for textboxes or the like. But r=me for this for now
> so we can get Fedora going.

That's a good point. I'll rework it slightly.
Marco, might as well test on win and mac eh?
Armen, can you give this a whirl?
Attachment #441223 - Attachment is obsolete: true
Attachment #441814 - Flags: review?(marco.zehe)
Attachment #441814 - Flags: feedback?(armenzg)
Comment on attachment 441814 [details] [diff] [review]
bypass linux only.

r=me for this approach.
Attachment #441814 - Flags: review?(marco.zehe) → review+
Comment on attachment 441814 [details] [diff] [review]
bypass linux only.

Gentleman, this works for me :)

14245 INFO Passed: 13995
14246 INFO Failed: 0
14247 INFO Todo:   7
14248 INFO SimpleTest FINISHED
Attachment #441814 - Flags: feedback?(armenzg) → feedback+
Landed on central: http://hg.mozilla.org/mozilla-central/rev/68d7be8a8375

Thanks all - I think we ended up with the right fix here.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.