Skip to content
This repository has been archived by the owner on Feb 26, 2022. It is now read-only.

Bug 741101 – Makes contentStyle* consistent with contentScript* about include match pattern #394

Merged
merged 1 commit into from Apr 10, 2012

Conversation

ZER0
Copy link
Contributor

@ZER0 ZER0 commented Apr 3, 2012

As described in my previous comment on bugzilla, that's actually don't solve the problem, but limit it, and makes the behavior consistent across contentStyle* and contentScript*.

else if (pattern.anyWebPage) {
documentRules.length = 0;
documentRules.push("regexp(\"^(https?|ftp)://.*?\")");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with this change, but feel bit worried that some add-on's may get broken by this. Could you write about this on mailing list to increase awareness and users to test this. If cfx testall passes all tests I'm r+ on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain it a bit more? What do you mean with "some add-ons may get broken by this"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently page mods using "*" would be applied to addon's pages shipped in data folder with your changes they no longer will be applied to them, so there is a risk of someone relying on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is a misunderstanding, this feature is not released yet so we're not going to break any add-on. Currently "_" for contentScript_are already limited to http/https/ftp, this patch makescontentStyle* consistent with that rule.

ZER0 added a commit that referenced this pull request Apr 10, 2012
@ZER0 ZER0 merged commit 6a1a82e into mozilla:master Apr 10, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants