WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 193686
Web Inspector: Audit: add supports key to test/group for compatibility
https://bugs.webkit.org/show_bug.cgi?id=193686
Summary
Web Inspector: Audit: add supports key to test/group for compatibility
Devin Rousso
Reported
2019-01-22 14:38:33 PST
If the test's/group's "supports" key is less than the minimum of the frontend and backend versions, the test is too old to be run and should be disabled for that inspection target. - if backend < supports < frontend, we can visualize results of the test, but some functionality within the test itself may not be available (e.g. the injected `WebInspectorAudit` object) - if frontend < supports < backend, we have all the test functionality, but may not be able to parse/visualize all of it - if backend <=> frontend < supports, we are fully supported - if supports < backend <=> frontend, we are completely out of date
Attachments
Patch
(24.06 KB, patch)
2019-01-23 13:02 PST
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
[Image] After Patch is applied
(771.93 KB, image/png)
2019-01-23 13:03 PST
,
Devin Rousso
no flags
Details
Patch
(24.06 KB, patch)
2019-01-23 15:45 PST
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2019-01-22 14:38:59 PST
<
rdar://problem/47460872
>
Devin Rousso
Comment 2
2019-01-23 13:02:12 PST
Created
attachment 359932
[details]
Patch
Devin Rousso
Comment 3
2019-01-23 13:03:02 PST
Created
attachment 359933
[details]
[Image] After Patch is applied This "fakely" makes one of the default audits have `supports: 0;` for the sake of testing/demonstration. The default audit is not actually being changed in this patch.
Devin Rousso
Comment 4
2019-01-23 14:40:16 PST
Comment on
attachment 359932
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=359932&action=review
> Source/WebInspectorUI/UserInterface/Models/AuditTestBase.js:49 > + if (this._supports < WI.AuditTestBase.Version) > + this._supported = false; > + else if (InspectorBackend.domains.Audit && this._supports < InspectorBackend.domains.Audit.VERSION) > + this._supported = false;
I just realized, I have these written backwards. It should be `this._supports > ...`.
Devin Rousso
Comment 5
2019-01-23 15:45:54 PST
Created
attachment 359965
[details]
Patch
Joseph Pecoraro
Comment 6
2019-01-24 17:33:49 PST
Comment on
attachment 359965
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=359965&action=review
r=me
> Source/WebInspectorUI/UserInterface/Models/AuditTestBase.js:52 > + if (!this.supported)
Nit: _supported when used inside this class since the getter is simple.
> Source/WebInspectorUI/UserInterface/Models/AuditTestBase.js:89 > + if (!this.supported)
Nit: _supported when used inside this class since the getter is simple.
Devin Rousso
Comment 7
2019-01-24 22:21:34 PST
Comment on
attachment 359965
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=359965&action=review
>> Source/WebInspectorUI/UserInterface/Models/AuditTestBase.js:52 >> + if (!this.supported) > > Nit: _supported when used inside this class since the getter is simple.
Considering that it's overridden by a subclass (`WI.AuditTestGroup`), I thought it best to use the non-member version. This is somewhat similar to what I did with `this.disabled`.
WebKit Commit Bot
Comment 8
2019-01-24 22:47:55 PST
Comment on
attachment 359965
[details]
Patch Clearing flags on attachment: 359965 Committed
r240469
: <
https://trac.webkit.org/changeset/240469
>
WebKit Commit Bot
Comment 9
2019-01-24 22:47:56 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug