1057176 - @-moz-document regexp('whatever') applies to browser tab corners

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Jason Barnabe (np)]

Firefox 31.

Add this with Stylish:

@-moz-document regexp('whatever')
{
* { background-color:red !important; }
}

Despite the regexp not matching the browser URL, this makes the browser's active tab corners have a red background. The tab corners are _moz_generated_content_before and _moz_generated_content_after, so maybe the regexp function doesn't work right with those? (This doesn't happen with non-matching domain, url, or url-prefix functions.)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screenshot

I don't see any effect on tab corners, but I do see it affecting the "x" close-buttons and the "gear" icon on the new-tab page.

34.0a1 (2014-08-21)
Mozilla/5.0 (X11; Linux x86_64; rv:34.0) Gecko/20100101 Firefox/34.0
Ubuntu 14.04

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: screenshot w/ tab corners affected as well

Ah, I do see it on the tab corners now. Not sure why that wasn't happening before or why it started happening.

Comment 3

[Boris Zbarsky [:bzbarsky]]

The regexp matcher for moz-document rules invokes the JS engine to do the work, which means it needs a script context to run in.

It uses the document being styled for this, and defaults to matching if there is no associated script context; see this part in nsContentUtils::IsPatternMatching:

   if (NS_WARN_IF(!jsapi.Init(aDocument->GetWindow()))) {
     return true;
   }

In particular, this will return true for cases when aDocument is an SVG image or SVG resource document...

This behavior for IsPatternMatching seems pretty bogus to me.  It should not be relying on the incoming document like that.

Note that this should observably affect HTML inputs with a validation pattern too, if they happen to be in a document that's not in a window (e.g. responseXML or other data document).

Comment 4

[Bob Owen (:bobowen)]

(In reply to Boris Zbarsky [:bz] from comment #3)

> It uses the document being styled for this, and defaults to matching if
> there is no associated script context; see this part in
> nsContentUtils::IsPatternMatching:
> 
>    if (NS_WARN_IF(!jsapi.Init(aDocument->GetWindow()))) {
>      return true;
>    }

This was using aDocument->GetWindow(), to get to the window's context, before we converted to AutoJSAPI, so I don't think the behaviour changed there.

However now that we're using the SafeJSContext, I think we could use aDocument->GetScopeObject().
This is probably the right thing to do anyway and would probably fix this assuming that the scope object is being set correctly.

[1] http://hg.mozilla.org/mozilla-central/diff/50dfea2bc887/content/base/src/nsContentUtils.cpp

Comment 5

[Boris Zbarsky [:bzbarsky]]

> so I don't think the behaviour changed there.

Sure.  It's been broken all along, since it first landed.

> I think we could use aDocument->GetScopeObject()

I expect that to be null for SVG images.

Comment 6

[Bob Owen (:bobowen)]

(In reply to Boris Zbarsky [:bz] from comment #5)
> > so I don't think the behaviour changed there.
> 
> Sure.  It's been broken all along, since it first landed.
> 
> > I think we could use aDocument->GetScopeObject()
> 
> I expect that to be null for SVG images.

Ah, right it seems this is the case sometimes, at least at [1].
aEventObject is used for the scope object.

I assume we will need some sort of scope to create the regexp object, so maybe this is another case where the unprivileged junk scope would make sense.
Given that this just returns a bool I don't suppose the scope matters much, but I guess we'll need to wait for Bobby to make a call on this.

[1] http://dxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxSVGGlyphs.cpp#355

Comment 7

[Bobby Holley (:bholley)]

Yeah, this seems like a good use-case for the unprivileged junk scope.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Don't try to use the document's window for regexp evaluation in nsContentUtils::IsPatternMatching, since there might not be one. Just use the unprivileged junk scope

Comment 9

[Bob Owen (:bobowen)]

Comment on attachment 8483891 [details] [diff] [review]
Don't try to use the document's window for regexp evaluation in nsContentUtils::IsPatternMatching, since there might not be one.  Just use the unprivileged junk scope

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

::: content/base/test/test_bug1057176.html
@@ +16,5 @@
> +  elem.pattern = "abc";
> +  elem.value = "def";
> +  ok(!elem.validity.valid, '"def" should not match the pattern "abc"');
> +  elem.value = "abc";
> +  ok(elem.validity.valid, '"abc" should not match the pattern "abc"');

Should this be "should match"?

Comment 10

[Boris Zbarsky [:bzbarsky]]

> Should this be "should match"?

Yes, it should.  Good catch!

Comment 11

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/52dffbc643c0

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/52dffbc643c0