The default bug view has changed. See this FAQ.

downloaded blocklist doesn't support unspecified version comparator

RESOLVED FIXED in mozilla15

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Joe Drew (not getting mail), Assigned: Joe Drew (not getting mail))

Tracking

Trunk
mozilla15
x86
Mac OS X
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 611904 [details] [diff] [review]
add support for no version comparison + tests

This caused the problems we had in 722538.
Attachment #611904 - Flags: review?(bjacob)

Comment 1

5 years ago
Try run for 0955a02c7bd8 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=0955a02c7bd8
Results (out of 29 total builds):
    success: 27
    warnings: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jdrew@mozilla.com-0955a02c7bd8
(Assignee)

Updated

5 years ago
Assignee: nobody → joe
Comment on attachment 611904 [details] [diff] [review]
add support for no version comparison + tests

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

::: toolkit/mozapps/extensions/test/xpcshell/data/test_gfxBlacklist.xml
@@ +119,5 @@
> +      <vendor>0xabcd</vendor>
> +      <devices>
> +        <device>0x6666</device>
> +      </devices>
> +      <feature> DIRECT2D </feature>

Do the spaces around DIRECT2D matter here, and below?

::: widget/xpwidgets/GfxInfoBase.cpp
@@ +691,5 @@
>        break;
>      case DRIVER_BETWEEN_INCLUSIVE_START:
>        match = driverVersion >= info[i].mDriverVersion && driverVersion < info[i].mDriverVersionMax;
>        break;
> +    case DRIVER_UNKNOWN_COMPARISON:

Why can't this be more explicitly names DRIVER_ALL_VERSIONS since that's what it does?
(Assignee)

Comment 3

5 years ago
(In reply to Benoit Jacob [:bjacob] from comment #2)
> Comment on attachment 611904 [details] [diff] [review]
> add support for no version comparison + tests
> 
> Review of attachment 611904 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/mozapps/extensions/test/xpcshell/data/test_gfxBlacklist.xml
> @@ +119,5 @@
> > +      <vendor>0xabcd</vendor>
> > +      <devices>
> > +        <device>0x6666</device>
> > +      </devices>
> > +      <feature> DIRECT2D </feature>
> 
> Do the spaces around DIRECT2D matter here, and below?

No. We explicitly trim all spaces before doing comparisons.

> ::: widget/xpwidgets/GfxInfoBase.cpp
> @@ +691,5 @@
> >        break;
> >      case DRIVER_BETWEEN_INCLUSIVE_START:
> >        match = driverVersion >= info[i].mDriverVersion && driverVersion < info[i].mDriverVersionMax;
> >        break;
> > +    case DRIVER_UNKNOWN_COMPARISON:
> 
> Why can't this be more explicitly names DRIVER_ALL_VERSIONS since that's
> what it does?

How about DRIVER_COMPARISON_IGNORED?
Works for me.
I still feel very incompetent to review the test. Is it a mochitest? it doesn't use mochitest stuff like ok() or todo().
ah ok, xpcshell test. can you find someone who knows that stuff? or teach me what i need to know,
Comment on attachment 611904 [details] [diff] [review]
add support for no version comparison + tests

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

r=me with constant name change discussed above, but consider the xpcshell test part as not very closely reviewed.
Attachment #611904 - Flags: review?(bjacob)
Attachment #611904 - Flags: review+
(Assignee)

Comment 8

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/f7cb2d403f33
https://hg.mozilla.org/mozilla-central/rev/f7cb2d403f33
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.