-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Reduce chromium noise #951
Changes from all commits
b0ca3a3
8d81329
53caf12
8b19b83
f070393
c3da66c
b3ba681
209a380
f0a849a
e366517
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,16 +4,14 @@ | |
using Axe.Windows.Core.Bases; | ||
using Axe.Windows.Core.Enums; | ||
using Axe.Windows.Rules.Resources; | ||
using static Axe.Windows.Rules.PropertyConditions.ControlType; | ||
using static Axe.Windows.Rules.PropertyConditions.Framework; | ||
using static Axe.Windows.Rules.PropertyConditions.Relationships; | ||
using static Axe.Windows.Rules.PropertyConditions.ElementGroups; | ||
|
||
namespace Axe.Windows.Rules.Library | ||
{ | ||
[RuleInfo(ID = RuleId.ChromiumComponentsShouldUseWebScanner)] | ||
class ChromiumComponentsShouldUseWebScanner : Rule | ||
{ | ||
public ChromiumComponentsShouldUseWebScanner() | ||
public ChromiumComponentsShouldUseWebScanner() : base(excludedCondition: null) | ||
{ | ||
Info.Description = Descriptions.ChromiumComponentsShouldUseWebScanner; | ||
Info.HowToFix = HowToFix.ChromiumComponentsShouldUseWebScanner; | ||
|
@@ -27,7 +25,7 @@ public override bool PassesTest(IA11yElement e) | |
|
||
protected override Condition CreateCondition() | ||
{ | ||
return Chrome & (Document | AnyAncestor(Document)); | ||
return IsChromiumDocument; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the change that limits |
||
} | ||
} // class | ||
} // namespace |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
using Axe.Windows.Core.Exceptions; | ||
using Axe.Windows.Core.Misc; | ||
using Axe.Windows.Rules.Resources; | ||
using static Axe.Windows.Rules.PropertyConditions.ElementGroups; | ||
|
||
namespace Axe.Windows.Rules | ||
{ | ||
|
@@ -19,17 +20,27 @@ abstract class Rule : IRule | |
{ | ||
public RuleInfo Info { get; private set; } | ||
public Condition Condition { get; } | ||
protected static Condition DefaultExcludedCondition => IsChromiumContent; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exposing this to derived classes gives us a single place to modify the default, then the production constructors of the derived classes just pick up the new value without requiring any additional code change |
||
|
||
protected Rule() | ||
protected Rule(Condition excludedCondition) | ||
{ | ||
// keep these two calls in the following order or the RuleInfo.Condition string won't get populated | ||
// keep these calls in the following order or the RuleInfo.Condition string won't get populated | ||
#pragma warning disable CA2214 | ||
Condition = CreateCondition(); | ||
#pragma warning restore CA2214 | ||
|
||
if (excludedCondition != null) | ||
{ | ||
Condition = Condition - excludedCondition; | ||
} | ||
|
||
InitRuleInfo(); | ||
} | ||
|
||
protected Rule() : this (excludedCondition: DefaultExcludedCondition) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By default, exclude |
||
{ | ||
} | ||
|
||
private void InitRuleInfo() | ||
{ | ||
var info = GetRuleInfoFromAttributes(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could build an argument for renaming this to
ChromiumDocumentsShouldUseWebScanner
, but that would make the PR harder to review. Happy to put this into a subsequent PR if needed.