Skip to content
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

avm2: Improve XMLList set_local_property #13255

Merged
merged 6 commits into from
Oct 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions core/src/avm2/activation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2140,22 +2140,22 @@ impl<'a, 'gc> Activation<'a, 'gc> {
) => {
let mut children = value1.children().clone();
children.push(E4XOrXml::Xml(value2));
XmlListObject::new(self, children, None).into()
XmlListObject::new(self, children, None, None).into()
}
(
Value::Object(Object::XmlObject(value1)),
Value::Object(Object::XmlListObject(value2)),
) => {
let mut children = vec![E4XOrXml::Xml(value1)];
children.extend(value2.children().clone());
XmlListObject::new(self, children, None).into()
XmlListObject::new(self, children, None, None).into()
}
(
Value::Object(Object::XmlObject(value1)),
Value::Object(Object::XmlObject(value2)),
) => {
let children = vec![E4XOrXml::Xml(value1), E4XOrXml::Xml(value2)];
XmlListObject::new(self, children, None).into()
XmlListObject::new(self, children, None, None).into()
}
(value1, value2) => {
let prim_value1 = value1.coerce_to_primitive(None, self)?;
Expand Down
12 changes: 8 additions & 4 deletions core/src/avm2/e4x.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,12 @@ impl<'gc> E4XNode<'gc> {
mc: &Mutation<'gc>,
namespace: Option<AvmString<'gc>>,
name: AvmString<'gc>,
parent: Self,
parent: Option<Self>,
) -> Self {
E4XNode(GcCell::new(
mc,
E4XNodeData {
parent: Some(parent),
parent,
namespace,
local_name: Some(name),
kind: E4XNodeKind::Element {
Expand All @@ -128,12 +128,12 @@ impl<'gc> E4XNode<'gc> {
mc: &Mutation<'gc>,
name: AvmString<'gc>,
value: AvmString<'gc>,
parent: E4XNode<'gc>,
parent: Option<E4XNode<'gc>>,
) -> Self {
E4XNode(GcCell::new(
mc,
E4XNodeData {
parent: Some(parent),
parent,
namespace: None,
local_name: Some(name),
kind: E4XNodeKind::Attribute(value),
Expand Down Expand Up @@ -824,6 +824,10 @@ impl<'gc> E4XNode<'gc> {
Ok(result)
}

pub fn set_namespace(&self, namespace: AvmString<'gc>, mc: &Mutation<'gc>) {
self.0.write(mc).namespace = Some(namespace);
}

pub fn namespace(&self) -> Option<AvmString<'gc>> {
self.0.read().namespace
}
Expand Down
47 changes: 36 additions & 11 deletions core/src/avm2/globals/xml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ pub fn child<'gc>(
} else {
Vec::new()
};
return Ok(XmlListObject::new(activation, children, None).into());
return Ok(XmlListObject::new(activation, children, None, None).into());
}
}

Expand All @@ -237,7 +237,8 @@ pub fn child<'gc>(
Vec::new()
};

Ok(XmlListObject::new(activation, children, Some(xml.into())).into())
// FIXME: If name is not a number index, then we should call [[Get]] (get_property_local) with the name.
Ok(XmlListObject::new(activation, children, Some(xml.into()), Some(multiname)).into())
}

pub fn child_index<'gc>(
Expand Down Expand Up @@ -281,7 +282,14 @@ pub fn children<'gc>(
Vec::new()
};

Ok(XmlListObject::new(activation, children, Some(xml.into())).into())
// FIXME: Spec says to just call [[Get]] with * (any multiname).
Ok(XmlListObject::new(
activation,
children,
Some(xml.into()),
Some(Multiname::any(activation.gc())),
)
.into())
}

pub fn copy<'gc>(
Expand Down Expand Up @@ -329,7 +337,7 @@ pub fn elements<'gc>(
Vec::new()
};

Ok(XmlListObject::new(activation, children, Some(xml.into())).into())
Ok(XmlListObject::new(activation, children, Some(xml.into()), None).into())
}

pub fn attributes<'gc>(
Expand All @@ -344,7 +352,14 @@ pub fn attributes<'gc>(
Vec::new()
};

Ok(XmlListObject::new(activation, attributes, Some(xml.into())).into())
// FIXME: Spec/avmplus says to call [[Get]] with * attribute name (any attribute multiname).
Ok(XmlListObject::new(
activation,
attributes,
Some(xml.into()),
Some(Multiname::any_attribute(activation.gc())),
)
.into())
}

pub fn attribute<'gc>(
Expand All @@ -364,7 +379,8 @@ pub fn attribute<'gc>(
Vec::new()
};

Ok(XmlListObject::new(activation, attributes, Some(xml.into())).into())
// FIXME: Spec/avmplus call [[Get]] with attribute name.
Ok(XmlListObject::new(activation, attributes, Some(xml.into()), Some(multiname)).into())
}

pub fn call_handler<'gc>(
Expand Down Expand Up @@ -461,7 +477,7 @@ pub fn append_child<'gc>(
activation.context.gc_context,
last_child_namespace,
last_child_name,
*xml.node(),
Some(*xml.node()),
); // Creating an element requires passing a parent node, unlike creating a text node

let text_node = E4XNode::text(activation.context.gc_context, text, None);
Expand Down Expand Up @@ -510,9 +526,10 @@ pub fn descendants<'gc>(
let multiname = name_to_multiname(activation, &args[0], false)?;
let mut descendants = Vec::new();
xml.node().descendants(&multiname, &mut descendants);
Ok(XmlListObject::new(activation, descendants, Some(xml.into())).into())
Ok(XmlListObject::new(activation, descendants, None, None).into())
}

// ECMA-357 13.4.4.37 XML.prototype.text ( )
pub fn text<'gc>(
activation: &mut Activation<'_, 'gc>,
this: Object<'gc>,
Expand All @@ -528,7 +545,9 @@ pub fn text<'gc>(
} else {
Vec::new()
};
Ok(XmlListObject::new(activation, nodes, Some(xml.into())).into())
// 1. Let list be a new XMLList with list.[[TargetObject]] = x and list.[[TargetProperty]] = null
// 3. Return list
Ok(XmlListObject::new(activation, nodes, Some(xml.into()), None).into())
}

pub fn length<'gc>(
Expand Down Expand Up @@ -559,6 +578,7 @@ pub fn has_simple_content<'gc>(
Ok(result.into())
}

// ECMA-357 13.4.4.9 XML.prototype.comments ( )
pub fn comments<'gc>(
activation: &mut Activation<'_, 'gc>,
this: Object<'gc>,
Expand All @@ -575,9 +595,12 @@ pub fn comments<'gc>(
Vec::new()
};

Ok(XmlListObject::new(activation, comments, Some(xml.into())).into())
// 1. Let list be a new XMLList with list.[[TargetObject]] = x and list.[[TargetProperty]] = null
// 3. Return list
Ok(XmlListObject::new(activation, comments, Some(xml.into()), None).into())
}

// ECMA-357 13.4.4.28 XML.prototype.processingInstructions ( [ name ] )
pub fn processing_instructions<'gc>(
activation: &mut Activation<'_, 'gc>,
this: Object<'gc>,
Expand All @@ -598,7 +621,9 @@ pub fn processing_instructions<'gc>(
Vec::new()
};

Ok(XmlListObject::new(activation, nodes, Some(xml.into())).into())
// 3. Let list = a new XMLList with list.[[TargetObject]] = x and list.[[TargetProperty]] = null
// 5. Return list
Ok(XmlListObject::new(activation, nodes, Some(xml.into()), None).into())
}

// ECMA-357 13.4.4.18 XML.prototype.insertChildAfter (child1, child2)
Expand Down
31 changes: 24 additions & 7 deletions core/src/avm2/globals/xml_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pub use crate::avm2::object::xml_list_allocator;
use crate::avm2::{
e4x::{name_to_multiname, simple_content_to_string, E4XNode, E4XNodeKind},
error::type_error,
multiname::Multiname,
object::{E4XOrXml, XmlListObject},
parameters::ParametersExt,
string::AvmString,
Expand Down Expand Up @@ -176,7 +177,7 @@ pub fn child<'gc>(
);
}
}
Ok(XmlListObject::new(activation, sub_children, Some(list.into())).into())
Ok(XmlListObject::new(activation, sub_children, Some(list.into()), None).into())
}

pub fn children<'gc>(
Expand All @@ -192,7 +193,14 @@ pub fn children<'gc>(
sub_children.extend(children.iter().map(|node| E4XOrXml::E4X(*node)));
}
}
Ok(XmlListObject::new(activation, sub_children, Some(list.into())).into())
// FIXME: This method should just call get_property_local with "*".
Ok(XmlListObject::new(
activation,
sub_children,
Some(list.into()),
Some(Multiname::any(activation.gc())),
)
.into())
}

pub fn copy<'gc>(
Expand Down Expand Up @@ -227,7 +235,9 @@ pub fn attribute<'gc>(
}
}
}
Ok(XmlListObject::new(activation, sub_children, Some(list.into())).into())

// FIXME: This should just use get_property_local with an attribute Multiname.
Ok(XmlListObject::new(activation, sub_children, Some(list.into()), Some(multiname)).into())
}

pub fn attributes<'gc>(
Expand All @@ -244,7 +254,14 @@ pub fn attributes<'gc>(
}
}

Ok(XmlListObject::new(activation, child_attrs, Some(list.into())).into())
// FIXME: This should just use get_property_local with an any attribute Multiname.
Ok(XmlListObject::new(
activation,
child_attrs,
Some(list.into()),
Some(Multiname::any_attribute(activation.gc())),
)
.into())
}

pub fn name<'gc>(
Expand Down Expand Up @@ -299,7 +316,7 @@ pub fn text<'gc>(
);
}
}
Ok(XmlListObject::new(activation, nodes, Some(xml_list.into())).into())
Ok(XmlListObject::new(activation, nodes, Some(xml_list.into()), None).into())
}

pub fn comments<'gc>(
Expand All @@ -319,7 +336,7 @@ pub fn comments<'gc>(
);
}
}
Ok(XmlListObject::new(activation, nodes, Some(xml_list.into())).into())
Ok(XmlListObject::new(activation, nodes, Some(xml_list.into()), None).into())
}

pub fn processing_instructions<'gc>(
Expand All @@ -344,5 +361,5 @@ pub fn processing_instructions<'gc>(
}
}

Ok(XmlListObject::new(activation, nodes, Some(xml_list.into())).into())
Ok(XmlListObject::new(activation, nodes, Some(xml_list.into()), None).into())
}
10 changes: 10 additions & 0 deletions core/src/avm2/multiname.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,16 @@ impl<'gc> Multiname<'gc> {
}
}

/// Indicates the any attribute type (any attribute in any namespace).
pub fn any_attribute(mc: &Mutation<'gc>) -> Self {
Self {
ns: NamespaceSet::single(Namespace::any(mc)),
name: None,
param: None,
flags: MultinameFlags::ATTRIBUTE,
}
}

pub fn new(ns: Namespace<'gc>, name: impl Into<AvmString<'gc>>) -> Self {
Self {
ns: NamespaceSet::single(ns),
Expand Down
Loading