nsContentUtils::IsCallerXBL misses <field>s

RESOLVED FIXED in mozilla20

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

unspecified
mozilla20
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
The semantics of xbl <field>s are a bit wacky, but they can nonetheless define functions, and currently when those get called we return the wrong answer for IsCallerXBL. Patch coming up.

Comment 1

6 years ago
Can you go into specifics about what the user impact would be here? I know this was called out specifically in relation to bug 810082.
tracking-firefox18: --- → ?
(Assignee)

Comment 2

6 years ago
(In reply to Alex Keybl [:akeybl] from comment #1)
> Can you go into specifics about what the user impact would be here? I know
> this was called out specifically in relation to bug 810082.

I added nsContentUtils::IsCallerXBL in bug 795275, so that the Components warning would not appear for XBL callers, where we're allowing Components access. However, someone went and backported that patch for bug 810082 and used it there. I don't have a great grasp of what that patch is trying to do, so I can't properly evaluate the risks of getting it wrong. John probably knows though. John, if IsCallerXBL sometimes returns false for XBL callers, is that a problem?

The other impact of course is that we might issue spurious warnings to the console for legit XBL Components access. But it might be kind of rare. Then again, this patch is extremely low risk. Let's see what John says.
Flags: needinfo?(jschoenick)
(Assignee)

Comment 3

6 years ago
Created attachment 691519 [details] [diff] [review]
Tag XBL script for <field> elements and child scripts. v1

Attaching a patch. Hopefully Waldo can tell me if there's any other way XBL code
can propagate to new scripts where we miss propagating userBit. Note that I
explictly _don't_ want to do it for Eval. ;-)
Attachment #691519 - Flags: review?(jwalden+bmo)
(In reply to Bobby Holley (:bholley) from comment #2)
> I added nsContentUtils::IsCallerXBL in bug 795275, so that the Components
> warning would not appear for XBL callers, where we're allowing Components
> access. However, someone went and backported that patch for bug 810082 and
> used it there. I don't have a great grasp of what that patch is trying to
> do, so I can't properly evaluate the risks of getting it wrong. John
> probably knows though. John, if IsCallerXBL sometimes returns false for XBL
> callers, is that a problem?

In bug 810082 we're trying to respond to an event when content JS touches a plugin tag that is in CTP mode. Such tags have an XBL element attached to them, which has a bit of JS in its constructor. If that XBL tag being attached triggers the event, then we would essentially always think content was trying to touch the tag.

A quick test shows that this particular XBL does not erroneously trigger the event, does that mean we safe from this bug or might it still trigger in some circumstances?
Flags: needinfo?(jschoenick)
(Assignee)

Comment 5

6 years ago
Comment on attachment 691519 [details] [diff] [review]
Tag XBL script for <field> elements and child scripts. v1

Jorendorff and I have been talking about this, so I'm going to pass this to him.
Attachment #691519 - Flags: review?(jwalden+bmo) → review?(jorendorff)
Comment on attachment 691519 [details] [diff] [review]
Tag XBL script for <field> elements and child scripts. v1

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

Looks perfect.
Attachment #691519 - Flags: review?(jorendorff) → review+
(Assignee)

Comment 7

6 years ago
Johns pointed me to pluginProblem.xml, which I verified doesn't contain <field>. So that bug should be unaffected.
https://hg.mozilla.org/mozilla-central/rev/46826841e0fe
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20

Updated

6 years ago
tracking-firefox18: ? → ---
Depends on: 821676
You need to log in before you can comment on or make changes to this bug.