632428 - "Fix the caller!" when datalist fires onselect on a node created from a chrome context

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect)

Description

[Marco Bonardo [:mak]]

I have a datalist in content context, chrome context fills it up with some <option> through a remote url that content doesn't have access to.

let list = contentDoc.getElementById("mydatalist");
let value = "xyz";
list.appendChild(
  list.ownerDocument.createElement("option")
).value = value;

when the datalist shows the node I created gets a onselect event, and this triggers a "WARNING: fix the caller!"

Smaug asked be to file the bug with a full stack

Comment 1

[Marco Bonardo [:mak]]

Attachment: stack at the warning

Comment 2

[Marco Bonardo [:mak]]

looking at local values I found a "onselect" in event userType and a "tree" in the target

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Seems like the not-so-safe AttributeChanged handling was added in Bug 556007.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

And this isn't only about AttributeChanged, but all the
mutation observer method.
We should probably use a script runner to call RevalidateDataList();

Comment 5

[Marco Bonardo [:mak]]

to reproduce the stack you can use "experiment 2.1" patch in bug 612453, it adds a autocomplete datalist to about:home search field. Just type in the search field to activate autocomplete and you'll see the warnings.

Comment 6

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

This patch is fixing the issue. I guess this is because OnUpdateSearchResult was called from nsIMutationObserver methods.

Comment 7

[Mounir Lamouri (:mounir)]

Why this bug has been marked "security-sensitive"?

Comment 8

[Marco Bonardo [:mak]]

Smaug asked me to mark it as security sensitive while it was investigated.

Comment 9

[Mounir Lamouri (:mounir)]

Attachment: Tests fixes

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #7)
> Why this bug has been marked "security-sensitive"?

Because nsAutoCompleteController::OnUpdateSearchResult is called at unsafe time, 
and that method seems to trigger some scripts to run.
If those script can include anything in content page, this would be probably
sg:critical.

Comment 11

[Mounir Lamouri (:mounir)]

(In reply to comment #10)
> (In reply to comment #7)
> > Why this bug has been marked "security-sensitive"?
> 
> Because nsAutoCompleteController::OnUpdateSearchResult is called at unsafe
> time, 
> and that method seems to trigger some scripts to run.
> If those script can include anything in content page, this would be probably
> sg:critical.

I don't think nsAutoCompleteController::OnUpdateSearchResult can trigger any content script.

Comment 12

[Johnny Stenback (:jst)]

sg:critical, just to be on the safe side. Let's get this landed!

Comment 13

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/23703ec8b73b
http://hg.mozilla.org/mozilla-central/rev/5a9359bc00f6