976923 - reftest fails - font-inflation disable-fontinfl-on-mobile-4.html

Status: RESOLVED FIXED

(Testing :: Reftest, defect)

Description

[Vincent Chen [:vichen]]

Precondition: B2G/OOP
For this case, it is not because font-inflation but test page scale to 2.5 and make two render result different.

Comment 1

[Abel Lin(alin, abel)]

Attachment: viewport.patch

The test created is from Bug 706198. I think it doesn't work on non-OOP and OOP.
Buy why it is still passed on non-oop. No matter what I set to viewport, there is no effect(font size unchanged), which means the rendering is always the same.

However, the value I set to viewport did work(font size changed) when enabling oop. I checked the reftest, I found it is hardcoded as 320. It seems like a mistake. It should be 'device-width' as ideal viewport as disable-fontinfl-on-mobile-ref.html(if app didn't assign a viewport, the ideal viewport would be set, Bug 940036 did it).

Anyway, The test should be corrected first.

As for the test on non-oop, I am not sure if it's normal because test is running in b2g. Does b2g ignore the viewport setting?

Comment 2

[Abel Lin(alin, abel)]

Hi Matt,
You reviewed the patch.
Could you comment it?

Comment 3

[Matt Brubeck (:mbrubeck)]

This change looks okay, but it might be better to just delete this test since it is now almost identical to disable-fontinfl-on-mobile-3.html.  I think the test was passing "by accident" before because some of the B2G viewport behavior depends on OOP-only code in TabChild/TabParent.  We may need to revise more of these tests if the default viewport behavior changes (see bug 946088).

Clearing the needinfo flag for :jwir3 since I don't think he's active involved these days.

Comment 4

[Abel Lin(alin, abel)]

(In reply to Matt Brubeck (:mbrubeck) from comment #3)
> This change looks okay, but it might be better to just delete this test
> since it is now almost identical to disable-fontinfl-on-mobile-3.html.  I
> think the test was passing "by accident" before because some of the B2G
> viewport behavior depends on OOP-only code in TabChild/TabParent.  We may
> need to revise more of these tests if the default viewport behavior changes
> (see bug 946088).
> 
> Clearing the needinfo flag for :jwir3 since I don't think he's active
> involved these days.

ok. The test case will be deleted if OOP enabled is ready.

Comment 5

[Vincent Chen [:vichen]]

(In reply to Abel Lin(alin, abel) from comment #4)
> (In reply to Matt Brubeck (:mbrubeck) from comment #3)
> > This change looks okay, but it might be better to just delete this test
> > since it is now almost identical to disable-fontinfl-on-mobile-3.html.  I
> > think the test was passing "by accident" before because some of the B2G
> > viewport behavior depends on OOP-only code in TabChild/TabParent.  We may
> > need to revise more of these tests if the default viewport behavior changes
> > (see bug 946088).
> > 
> > Clearing the needinfo flag for :jwir3 since I don't think he's active
> > involved these days.
> 
> ok. The test case will be deleted if OOP enabled is ready.

Matt,
OOP is enabled. Would you please check to keep this test or not?

Comment 6

[Matt Brubeck (:mbrubeck)]

Yes, I think we should delete the "disable-fontinfl-on-mobile-4.html" reftest.  It assumes that the default viewport for the reference file is 320px, which is no longer correct in general.

Comment 7

[u459114]

Attachment: Remove out-of-date test case

Comment 8

[u459114]

Hi Matt,
According to comment 6, I did this change. Please help to review, thanks

Comment 9

[u459114]

Attachment: Remove out-of-date test case

Hi Matt,
According to comment 6, I did this change. Please help to review, thanks

Comment 10

[u459114]

Try result
https://tbpl.mozilla.org/?tree=Try&rev=d3fb58990efc

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/39d59ff8a543

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/39d59ff8a543