Skip to content

Commit a86b2f2

Browse files
author
bors-servo
authored
Auto merge of #19904 - emilio:window-named-getter, r=<try>
[WIP] script: Partially implement the window named getter. There's tons of layout tests that depend on this, and I keep forgetting Servo doesn't support it :) <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19904) <!-- Reviewable:end -->
2 parents 76b4e5c + feeb833 commit a86b2f2

File tree

5 files changed

+132
-78
lines changed

5 files changed

+132
-78
lines changed

‎components/script/dom/bindings/codegen/CodegenRust.py‎

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2673,8 +2673,6 @@ def define(self):
26732673
interface.getUserData("hasProxyDescendant", False)):
26742674
depth = self.descriptor.prototypeDepth
26752675
check = "class.interface_chain[%s] == PrototypeList::ID::%s" % (depth, name)
2676-
elif self.descriptor.proxy:
2677-
check = "class as *const _ == &Class as *const _"
26782676
else:
26792677
check = "class as *const _ == &Class.dom_class as *const _"
26802678
return """\
@@ -5479,15 +5477,6 @@ def generate_code(self):
54795477
return CGGeneric(finalizeHook(self.descriptor, self.name, self.args[0].name))
54805478

54815479

5482-
class CGDOMJSProxyHandlerDOMClass(CGThing):
5483-
def __init__(self, descriptor):
5484-
CGThing.__init__(self)
5485-
self.descriptor = descriptor
5486-
5487-
def define(self):
5488-
return "static Class: DOMClass = " + DOMClass(self.descriptor) + ";\n"
5489-
5490-
54915480
class CGInterfaceTrait(CGThing):
54925481
def __init__(self, descriptor):
54935482
CGThing.__init__(self)
@@ -5917,10 +5906,10 @@ def reexportedName(name):
59175906
properties = PropertyArrays(descriptor)
59185907

59195908
if descriptor.concrete:
5909+
cgThings.append(CGDOMJSClass(descriptor))
59205910
if descriptor.proxy:
59215911
# cgThings.append(CGProxyIsProxy(descriptor))
59225912
cgThings.append(CGProxyUnwrap(descriptor))
5923-
cgThings.append(CGDOMJSProxyHandlerDOMClass(descriptor))
59245913
cgThings.append(CGDOMJSProxyHandler_ownPropertyKeys(descriptor))
59255914
if descriptor.interface.getExtendedAttribute("LegacyUnenumerableNamedProperties"):
59265915
cgThings.append(CGDOMJSProxyHandler_getOwnEnumerablePropertyKeys(descriptor))
@@ -5940,10 +5929,6 @@ def reexportedName(name):
59405929

59415930
# cgThings.append(CGDOMJSProxyHandler(descriptor))
59425931
# cgThings.append(CGIsMethod(descriptor))
5943-
pass
5944-
else:
5945-
cgThings.append(CGDOMJSClass(descriptor))
5946-
pass
59475932

59485933
if descriptor.isGlobal():
59495934
cgThings.append(CGWrapGlobalMethod(descriptor, properties))

‎components/script/dom/document.rs‎

Lines changed: 70 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,56 @@ pub struct Document {
368368
throw_on_dynamic_markup_insertion_counter: Cell<u64>,
369369
}
370370

371+
// https://html.spec.whatwg.org/multipage/#dom-document-nameditem-filter
372+
fn filter_by_name(name: &Atom, element: &Element) -> bool {
373+
// TODO: Handle <embed>, <iframe> and <object>.
374+
if element.is::<HTMLFormElement>() || element.is::<HTMLImageElement>() {
375+
if let Some(attr) = element.get_attribute(&ns!(), &local_name!("name")) {
376+
if attr.value().as_atom() == name {
377+
return true;
378+
}
379+
}
380+
}
381+
382+
if element.is::<HTMLImageElement>() &&
383+
element.get_id().as_ref().map_or(false, |id| id == name)
384+
{
385+
return true;
386+
}
387+
388+
false
389+
}
390+
391+
/// The filter for the named getter HTMLCollection that must be returned from
392+
/// the document.
393+
#[derive(JSTraceable, MallocSizeOf)]
394+
struct NamedGetterFilter {
395+
name: Atom,
396+
}
397+
398+
impl CollectionFilter for NamedGetterFilter {
399+
fn filter(&self, elem: &Element, _root: &Node) -> bool {
400+
filter_by_name(&self.name, elem)
401+
}
402+
}
403+
404+
/// The filter for the named getter HTMLCollection that must be returned from
405+
/// the window (not the document).
406+
#[derive(JSTraceable, MallocSizeOf)]
407+
pub struct WindowNamedGetterFilter {
408+
pub name: Atom,
409+
}
410+
411+
impl CollectionFilter for WindowNamedGetterFilter {
412+
fn filter(&self, element: &Element, _: &Node) -> bool {
413+
if filter_by_name(&self.name, element) {
414+
return true;
415+
}
416+
417+
element.get_id().as_ref().map_or(false, |id| id == &self.name)
418+
}
419+
}
420+
371421
#[derive(JSTraceable, MallocSizeOf)]
372422
struct ImagesFilter;
373423
impl CollectionFilter for ImagesFilter {
@@ -3555,79 +3605,40 @@ impl DocumentMethods for Document {
35553605
#[allow(unsafe_code)]
35563606
// https://html.spec.whatwg.org/multipage/#dom-tree-accessors:dom-document-nameditem-filter
35573607
unsafe fn NamedGetter(&self, _cx: *mut JSContext, name: DOMString) -> Option<NonNull<JSObject>> {
3558-
#[derive(JSTraceable, MallocSizeOf)]
3559-
struct NamedElementFilter {
3560-
name: Atom,
3561-
}
3562-
impl CollectionFilter for NamedElementFilter {
3563-
fn filter(&self, elem: &Element, _root: &Node) -> bool {
3564-
filter_by_name(&self.name, elem.upcast())
3565-
}
3566-
}
3567-
// https://html.spec.whatwg.org/multipage/#dom-document-nameditem-filter
3568-
fn filter_by_name(name: &Atom, node: &Node) -> bool {
3569-
let html_elem_type = match node.type_id() {
3570-
NodeTypeId::Element(ElementTypeId::HTMLElement(type_)) => type_,
3571-
_ => return false,
3572-
};
3573-
let elem = match node.downcast::<Element>() {
3574-
Some(elem) => elem,
3575-
None => return false,
3576-
};
3577-
match html_elem_type {
3578-
HTMLElementTypeId::HTMLFormElement => {
3579-
match elem.get_attribute(&ns!(), &local_name!("name")) {
3580-
Some(ref attr) => attr.value().as_atom() == name,
3581-
None => false,
3582-
}
3583-
},
3584-
HTMLElementTypeId::HTMLImageElement => {
3585-
match elem.get_attribute(&ns!(), &local_name!("name")) {
3586-
Some(ref attr) => {
3587-
if attr.value().as_atom() == name {
3588-
true
3589-
} else {
3590-
match elem.get_attribute(&ns!(), &local_name!("id")) {
3591-
Some(ref attr) => attr.value().as_atom() == name,
3592-
None => false,
3593-
}
3594-
}
3595-
},
3596-
None => false,
3597-
}
3598-
},
3599-
// TODO: Handle <embed>, <iframe> and <object>.
3600-
_ => false,
3601-
}
3602-
}
36033608
let name = Atom::from(name);
3609+
let filter = NamedGetterFilter { name };
3610+
36043611
let root = self.upcast::<Node>();
36053612
{
36063613
// Step 1.
3607-
let mut elements = root.traverse_preorder()
3608-
.filter(|node| filter_by_name(&name, &node))
3609-
.peekable();
3610-
if let Some(first) = elements.next() {
3611-
if elements.peek().is_none() {
3612-
// TODO: Step 2.
3613-
// Step 3.
3614-
return Some(NonNull::new_unchecked(first.reflector().get_jsobject().get()));
3615-
}
3616-
} else {
3617-
return None;
3614+
let mut elements = root
3615+
.traverse_preorder()
3616+
.filter_map(DomRoot::downcast::<Element>)
3617+
.filter(|element| filter.filter(&element, &root));
3618+
3619+
let first_element = elements.next()?;
3620+
if elements.next().is_none() {
3621+
// TODO: Step 2.
3622+
// Step 3.
3623+
return Some(NonNull::new_unchecked(
3624+
first_element.reflector().get_jsobject().get()
3625+
));
36183626
}
36193627
}
36203628
// Step 4.
3621-
let filter = NamedElementFilter {
3622-
name: name,
3623-
};
36243629
let collection = HTMLCollection::create(self.window(), root, Box::new(filter));
36253630
Some(NonNull::new_unchecked(collection.reflector().get_jsobject().get()))
36263631
}
36273632

36283633
// https://html.spec.whatwg.org/multipage/#dom-tree-accessors:supported-property-names
36293634
fn SupportedPropertyNames(&self) -> Vec<DOMString> {
36303635
// FIXME: unimplemented (https://github.com/servo/servo/issues/7273)
3636+
//
3637+
// We really need a HashMap from name to element, much like `id_map` for
3638+
// this.
3639+
//
3640+
// Chances are that if you implement this, you can also implement most
3641+
// of Window::SupportedPropertyNames.
36313642
vec![]
36323643
}
36333644

‎components/script/dom/element.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,10 @@ impl Element {
303303
*self.is.borrow_mut() = Some(is);
304304
}
305305

306+
pub fn get_id(&self) -> Ref<Option<Atom>> {
307+
self.id_attribute.borrow()
308+
}
309+
306310
pub fn get_is(&self) -> Option<LocalName> {
307311
self.is.borrow().clone()
308312
}

‎components/script/dom/webidls/Window.webidl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
//getter WindowProxy (unsigned long index);
4747

4848
// https://github.com/servo/servo/issues/14453
49-
// getter object (DOMString name);
49+
getter object (DOMString name);
5050

5151
// the user agent
5252
readonly attribute Navigator navigator;

‎components/script/dom/window.rs‎

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,12 @@ use dom::bluetooth::BluetoothExtraPermissionData;
3030
use dom::crypto::Crypto;
3131
use dom::cssstyledeclaration::{CSSModificationAccess, CSSStyleDeclaration, CSSStyleOwner};
3232
use dom::customelementregistry::CustomElementRegistry;
33-
use dom::document::{AnimationFrameCallback, Document};
33+
use dom::document::{AnimationFrameCallback, Document, WindowNamedGetterFilter};
3434
use dom::element::Element;
3535
use dom::event::Event;
3636
use dom::globalscope::GlobalScope;
3737
use dom::history::History;
38+
use dom::htmlcollection::{CollectionFilter, HTMLCollection};
3839
use dom::htmliframeelement::build_mozbrowser_custom_event;
3940
use dom::location::Location;
4041
use dom::mediaquerylist::{MediaQueryList, WeakMediaQueryListVec};
@@ -54,7 +55,7 @@ use euclid::{Point2D, Vector2D, Rect, Size2D};
5455
use fetch;
5556
use ipc_channel::ipc::{self, IpcSender};
5657
use ipc_channel::router::ROUTER;
57-
use js::jsapi::{HandleObject, HandleValue, JSAutoCompartment, JSContext};
58+
use js::jsapi::{HandleObject, HandleValue, JSAutoCompartment, JSContext, JSObject};
5859
use js::jsapi::{JS_GC, JS_GetRuntime};
5960
use js::jsval::UndefinedValue;
6061
use layout_image::fetch_image_for_layout;
@@ -82,6 +83,7 @@ use script_traits::{TimerSchedulerMsg, UntrustedNodeAddress, WindowSizeData, Win
8283
use script_traits::webdriver_msg::{WebDriverJSError, WebDriverJSResult};
8384
use selectors::attr::CaseSensitivity;
8485
use servo_arc;
86+
use servo_atoms::Atom;
8587
use servo_config::opts;
8688
use servo_config::prefs::PREFS;
8789
use servo_geometry::{f32_rect_to_au_rect, MaxRect};
@@ -95,6 +97,7 @@ use std::env;
9597
use std::fs;
9698
use std::io::{Write, stderr, stdout};
9799
use std::mem;
100+
use std::ptr::NonNull;
98101
use std::rc::Rc;
99102
use std::sync::{Arc, Mutex};
100103
use std::sync::atomic::{AtomicBool, Ordering};
@@ -842,6 +845,56 @@ impl WindowMethods for Window {
842845
CSSModificationAccess::Readonly)
843846
}
844847

848+
// https://html.spec.whatwg.org/multipage/window-object.html#named-access-on-the-window-object
849+
fn SupportedPropertyNames(&self) -> Vec<DOMString> {
850+
// FIXME: unimplemented (https://github.com/servo/servo/issues/7273).
851+
//
852+
// See also Document::SupportedPropertyNames.
853+
vec![]
854+
}
855+
856+
#[allow(unsafe_code)]
857+
// https://html.spec.whatwg.org/multipage/#named-access-on-the-window-object
858+
unsafe fn NamedGetter(
859+
&self,
860+
_cx: *mut JSContext,
861+
name: DOMString,
862+
) -> Option<NonNull<JSObject>> {
863+
// TODO: Return child windows, like iframes and such. When this is
864+
// fixed, please implement IndexedGetter properly too.
865+
let document = self.Document();
866+
if !document.is_html_document() {
867+
return None;
868+
}
869+
870+
// TODO(emilio): There is a missing fast-path here for when we know
871+
// there aren't any named items with a given `name`. In that case, we
872+
// can return `document.get_element_by_id(name)` (unless there are many
873+
// of those elements), which is much faster.
874+
let name = Atom::from(name);
875+
876+
let filter = WindowNamedGetterFilter { name };
877+
878+
let root = document.upcast();
879+
{
880+
let mut named_elements = document.upcast::<Node>()
881+
.traverse_preorder()
882+
.filter_map(DomRoot::downcast::<Element>)
883+
.filter(|element| filter.filter(&element, &root));
884+
885+
let first_element = named_elements.next()?;
886+
if named_elements.next().is_none() {
887+
return Some(NonNull::new_unchecked(
888+
first_element.reflector().get_jsobject().get()
889+
));
890+
}
891+
}
892+
893+
let collection =
894+
HTMLCollection::create(self, root, Box::new(filter));
895+
Some(NonNull::new_unchecked(collection.reflector().get_jsobject().get()))
896+
}
897+
845898
// https://drafts.csswg.org/cssom-view/#dom-window-innerheight
846899
//TODO Include Scrollbar
847900
fn InnerHeight(&self) -> i32 {
@@ -1609,6 +1662,7 @@ impl Window {
16091662

16101663
// https://html.spec.whatwg.org/multipage/#accessing-other-browsing-contexts
16111664
pub fn IndexedGetter(&self, _index: u32, _found: &mut bool) -> Option<DomRoot<Window>> {
1665+
// TODO: When this is fixed, also implement NamedGetter properly.
16121666
None
16131667
}
16141668

0 commit comments

Comments
 (0)