Shrink selectors::Component, implement attr selector (in)case-sensitivity #16915

SimonSapin · 2017-05-17T20:24:40Z

This change is

highfive · 2017-05-17T20:24:43Z

Heads up! This PR modifies the following files:

@bholley: components/style/restyle_hints.rs, components/style/servo/selector_parser.rs, components/style/attr.rs, components/style/gecko/snapshot.rs, components/style/gecko/wrapper.rs and 1 more
@KiChjang: components/script_layout_interface/wrapper_traits.rs, components/script/dom/element.rs, components/script/layout_wrapper.rs
@fitzgen: components/script_layout_interface/wrapper_traits.rs, components/script/dom/element.rs, components/script/layout_wrapper.rs
@emilio: components/style/restyle_hints.rs, components/style/servo/selector_parser.rs, components/style/attr.rs, components/style/gecko/snapshot.rs, components/style/gecko/wrapper.rs and 1 more

highfive · 2017-05-17T20:24:45Z

Warning

These commits modify unsafe code. Please review it carefully!

SimonSapin · 2017-05-17T20:37:50Z

r? @emilio

@bors-servo try

Gecko try on top of autoland, since central conflicts: https://treeherder.mozilla.org/#/jobs?repo=try&revision=98cf044a98e340788faeac76d8939a2fc85b521d

bors-servo · 2017-05-17T20:37:54Z

⌛ Trying commit f3b86e1 with merge e6d60a1...


        Auto merge of #16915 - servo:attr-selectors, r=<try>

Make selectors::Component smaller, add case-insensitive for other attr selectors * https://bugzilla.mozilla.org/show_bug.cgi?id=1364148 * https://bugzilla.mozilla.org/show_bug.cgi?id=1364162  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16915)

bors-servo · 2017-05-17T21:13:11Z

💔 Test failed - linux-rel-wpt

SimonSapin · 2017-05-17T21:20:07Z

Looks like a legit failure:

  ▶ CRASH [expected OK] /dom/nodes/ParentNode-querySelector-All.html
  │ 
  │ VMware, Inc.
  │ Gallium 0.4 on softpipe
  │ 3.3 (Core Profile) Mesa 12.0.1
  │ byte index 5 is not a char boundary; it is inside '北' (bytes 3..6) of `台北Táiběi 台北` (thread ScriptThread PipelineId { namespace_id: PipelineNamespaceId(0), index: PipelineIndex(1) }, at /checkout/src/libcore/str/mod.rs:2182)
  │ stack backtrace:
  │    0:     0x7f9e20baa4dc - backtrace::backtrace::trace::haa53f4880ff627c6
  │    1:     0x7f9e20baa9a2 - backtrace::capture::Backtrace::new::h5f3de869c0349b3d
  │    2:     0x7f9e1f77caf5 - servo::main::{{closure}}::hc2262116bcc3d90c
  │    3:     0x7f9e217ae8ca - std::panicking::rust_panic_with_hook
  │                         at /checkout/src/libstd/panicking.rs:550
  │    4:     0x7f9e217ae714 - std::panicking::begin_panic<collections::string::String>
  │                         at /checkout/src/libstd/panicking.rs:511
  │    5:     0x7f9e217ae699 - std::panicking::begin_panic_fmt
  │                         at /checkout/src/libstd/panicking.rs:495
  │    6:     0x7f9e217ae627 - std::panicking::rust_begin_panic
  │                         at /checkout/src/libstd/panicking.rs:471
  │    7:     0x7f9e217da30d - core::panicking::panic_fmt
  │                         at /checkout/src/libcore/panicking.rs:69
  │    8:     0x7f9e217db329 - core::str::slice_error_fail
  │                         at /checkout/src/libcore/str/mod.rs:2182
  │    9:     0x7f9e2138211a - selectors::attr::AttrSelectorOperator::eval_str::h71efd5a83ee2149e
  │   10:     0x7f9e1fdee9ca - script::dom::element::<impl selectors::tree::Element for script::dom::bindings::js::Root<script::dom::element::Element>>::attr_matches::hd155031e5697ced4
  │   11:     0x7f9e1fce0b47 - selectors::matching::matches_simple_selector::h221067263eb7b83b
  │   12:     0x7f9e1fce1e0f - selectors::matching::matches_complex_selector_internal::h82a493d67a9645ec
  │   13:     0x7f9e1fcd7bc0 - <core::slice::Iter<'a, T> as core::iter::iterator::Iterator>::all::{{closure}}::hf51aac52eb273d9b
  │   14:     0x7f9e1fce056a - selectors::matching::matches_selector_list::h2a8bb6823815c71a
  │   15:     0x7f9e1fea16b8 - <script::dom::node::QuerySelectorIterator as core::iter::iterator::Iterator>::next::hb7524d8c77e9d9a0
  │   16:     0x7f9e1fcaadc6 - <core::iter::Map<I, F> as core::iter::iterator::Iterator>::next::hcb58b03ffa8d0a8e
  │   17:     0x7f9e1fea59a3 - script::dom::node::Node::query_selector_all::he1c39e7a30fac3eb
  │   18:     0x7f9e1fb5958d - std::panicking::try::do_call::hb3ccf44d2235eb3d
  │   19:     0x7f9e217b59ea - panic_unwind::__rust_maybe_catch_panic
  │                         at /checkout/src/libpanic_unwind/lib.rs:98
  │   20:     0x7f9e200c6ef7 - script::dom::bindings::codegen::Bindings::DocumentBinding::DocumentBinding::querySelectorAll::h1181ec7db53bdfc9
  │   21:     0x7f9e202d7414 - CallJitMethodOp
  │   22:     0x7f9e1fd3bcbc - script::dom::bindings::utils::generic_call::h86cbfedc7c4ee9ad
  │   23:     0x7f9e20633320 - CallJSNative
  │                         at /home/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/834ce35/mozjs/js/src/jscntxtinlines.h:232
  │                          - 2js23InternalCallOrConstructEP9JSContextRKN2JS8CallArgsENS_14MaybeConstruct
  │                         at /home/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/834ce35/mozjs/js/src/vm/Interpreter.cpp:453
  │   24:     0x7f9e206334d4 - 2js4CallEP9JSContextN2JS6HandleINS2_5ValueEEES5_RKNS_13AnyInvokeArgsENS2_13MutableHandleIS4_E
  │                         at /home/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/834ce35/mozjs/js/src/vm/Interpreter.cpp:517
  │   25:     0x7f9e20595f47 - K2js7Wrapper4callEP9JSContextN2JS6HandleIP8JSObjectEERKNS3_8CallArgs
  │                         at /home/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/834ce35/mozjs/js/src/proxy/Wrapper.cpp:165
  │   26:     0x7f9e20588ac8 - K2js23CrossCompartmentWrapper4callEP9JSContextN2JS6HandleIP8JSObjectEERKNS3_8CallArgs
  │                         at /home/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/834ce35/mozjs/js/src/proxy/CrossCompartmentWrapper.cpp:329
  │   27:     0x7f9e2058f596 - 2js5Proxy4callEP9JSContextN2JS6HandleIP8JSObjectEERKNS3_8CallArgs
  │                         at /home/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/834ce35/mozjs/js/src/proxy/Proxy.cpp:401
  │   28:     0x7f9e2058ffab - 2js10proxy_CallEP9JSContextjPN2JS5Value
  │                         at /home/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/834ce35/mozjs/js/src/proxy/Proxy.cpp:689
  │   29:     0x7f9e206333ce - CallJSNative
  │                         at /home/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/834ce35/mozjs/js/src/jscntxtinlines.h:232
  │                          - 2js23InternalCallOrConstructEP9JSContextRKN2JS8CallArgsENS_14MaybeConstruct
  │                         at /home/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/834ce35/mozjs/js/src/vm/Interpreter.cpp:441
  │   30:     0x7f9e20305f64 - DoCallFallback
  │                         at /home/servo/.cargo/git/checkouts/mozjs-fa11ffc7d4f1cc2d/834ce35/mozjs/js/src/jit/BaselineIC.cpp:5970
  │   31:     0x7f9e221f9f58 - <unknown>
  │ ERROR:servo: byte index 5 is not a char boundary; it is inside '北' (bytes 3..6) of `台北Táiběi 台北`
  └ Pipeline failed in hard-fail mode.  Crashing!

SimonSapin · 2017-05-17T21:42:44Z

Slicing [u8] instead of str.

@bors-servo try

bors-servo · 2017-05-17T21:42:49Z

⌛ Trying commit 4585067 with merge 7a7dbad...


        Auto merge of #16915 - servo:attr-selectors, r=<try>

Make selectors::Component smaller, add case-insensitive for other attr selectors * https://bugzilla.mozilla.org/show_bug.cgi?id=1364148 * https://bugzilla.mozilla.org/show_bug.cgi?id=1364162  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16915)

SimonSapin · 2017-05-17T21:44:56Z

Gecko expectations:

diff --git a/layout/style/test/stylo-failures.md b/layout/style/test/stylo-failures.md
index 20fa0989a66e..714081e90908 100644
--- a/layout/style/test/stylo-failures.md
+++ b/layout/style/test/stylo-failures.md
@@ -90,7 +90,7 @@ to mochitest command.
   * ... `font-feature-settings`: bug 1355366 [10]
 * test_font_face_parser.html `font-weight`: keyword values should be preserved in \@font-face [4]
 * @namespace support:
-  * test_namespace_rule.html: bug 1355715 [8]
+  * test_namespace_rule.html: bug 1355715 [6]
 * test_dont_use_document_colors.html: support of disabling document color bug 1355716 [21]
 * test_font_feature_values_parsing.html: \@font-feature-values support bug 1355721 [107]
 * Grid support bug 1341802

bors-servo · 2017-05-17T22:28:37Z

💔 Test failed - mac-dev-unit

SimonSapin · 2017-05-17T22:30:22Z

@bors-servo try retry #16644

bors-servo · 2017-05-17T22:30:25Z

⌛ Trying commit 4585067 with merge 33c7477...


        Auto merge of #16915 - servo:attr-selectors, r=<try>

Make selectors::Component smaller, add case-insensitive for other attr selectors * https://bugzilla.mozilla.org/show_bug.cgi?id=1364148 * https://bugzilla.mozilla.org/show_bug.cgi?id=1364162  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16915)

bors-servo · 2017-05-18T00:35:04Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev
State: approved= try=True

SimonSapin · 2017-05-18T07:00:03Z

Added more changes: case-insensitive values, removing each_class.

@bors-servo try

https://treeherder.mozilla.org/#/jobs?repo=try&revision=35ae90c9d51a07e5c267c470cc8f7788797d8f23


        Add more enum types to avoid unreachable!() for selector case-sensiti…

…vity


        Short-circuit and unindent


        Don’t visit attribute selectors that never match.

SimonSapin · 2017-05-18T15:13:55Z

Missed the expectation change for the XHTML version of the WPT test that was fixed.

@bors-servo r=nox,emilio

bors-servo · 2017-05-18T15:13:55Z

📌 Commit c968842 has been approved by nox,emilio

SimonSapin · 2017-05-18T17:16:36Z

@bors-servo p=10

bors-servo · 2017-05-18T21:28:23Z

⌛ Testing commit c968842 with merge d5e2f91...


        Auto merge of #16915 - servo:attr-selectors, r=nox,emilio

Shrink selectors::Component, implement attr selector (in)case-sensitivity * https://bugzilla.mozilla.org/show_bug.cgi?id=1364148 * https://bugzilla.mozilla.org/show_bug.cgi?id=1364162 * https://bugzilla.mozilla.org/show_bug.cgi?id=1363531  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16915)

bors-servo · 2017-05-18T22:09:13Z

💔 Test failed - linux-rel-css


        More test expectations changes…

SimonSapin · 2017-05-18T22:21:34Z

@bors-servo r=nox,emilio

bors-servo · 2017-05-18T22:21:35Z

📌 Commit b359e3f has been approved by nox,emilio

bors-servo · 2017-05-18T23:45:22Z

⌛ Testing commit b359e3f with merge 640b166...


        Auto merge of #16915 - servo:attr-selectors, r=nox,emilio

Shrink selectors::Component, implement attr selector (in)case-sensitivity * https://bugzilla.mozilla.org/show_bug.cgi?id=1364148 * https://bugzilla.mozilla.org/show_bug.cgi?id=1364162 * https://bugzilla.mozilla.org/show_bug.cgi?id=1363531 * Fixes #3322  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16915)

bors-servo · 2017-05-19T01:42:51Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev
Approved by: nox,emilio
Pushing 640b166 to master...

highfive assigned mbrubeck May 17, 2017

highfive added the S-awaiting-review label May 17, 2017

SimonSapin force-pushed the attr-selectors branch from 80811b8 to f3b86e1 May 17, 2017

highfive assigned emilio and unassigned mbrubeck May 17, 2017

highfive added the S-tests-failed label May 17, 2017

SimonSapin force-pushed the attr-selectors branch from f3b86e1 to 4585067 May 17, 2017

highfive removed the S-tests-failed label May 17, 2017

highfive added the S-tests-failed label May 17, 2017

SimonSapin changed the title ~~Make selectors::Component smaller, add case-insensitive for other attr selectors~~ Shrink selectors::Component, implement attr selector (in)case-sensitivity May 18, 2017

highfive removed the S-tests-failed label May 18, 2017

SimonSapin force-pushed the attr-selectors branch 2 times, most recently from a157f88 to bad8b91 May 18, 2017

SimonSapin added 3 commits May 18, 2017

Add more enum types to avoid unreachable!() for selector case-sensiti…

76166bc

…vity

Short-circuit and unindent

c06e574

Don’t visit attribute selectors that never match.

Loading status checks…

c968842

SimonSapin force-pushed the attr-selectors branch from 029bbe0 to c968842 May 18, 2017

highfive added S-awaiting-review and removed S-tests-failed labels May 18, 2017

highfive added S-awaiting-merge and removed S-awaiting-review labels May 18, 2017

highfive added S-tests-failed and removed S-awaiting-merge labels May 18, 2017

More test expectations changes…

Loading status checks…

b359e3f

highfive added S-awaiting-review and removed S-tests-failed labels May 18, 2017

highfive added S-awaiting-merge and removed S-awaiting-review labels May 18, 2017

highfive removed the S-awaiting-merge label May 19, 2017

SimonSapin deleted the attr-selectors branch May 19, 2017

upsuper mentioned this pull request May 24, 2017

Attribute selector should treat values of some attributes as case-insensitive #15006

Closed

servo / servo

Shrink selectors::Component, implement attr selector (in)case-sensitivity #16915

Shrink selectors::Component, implement attr selector (in)case-sensitivity #16915

SimonSapin commented May 17, 2017 •

edited

highfive commented May 17, 2017

highfive commented May 17, 2017

SimonSapin commented May 17, 2017

bors-servo commented May 17, 2017

bors-servo commented May 17, 2017

SimonSapin commented May 17, 2017

SimonSapin commented May 17, 2017

bors-servo commented May 17, 2017

SimonSapin commented May 17, 2017

bors-servo commented May 17, 2017

SimonSapin commented May 17, 2017

bors-servo commented May 17, 2017

bors-servo commented May 18, 2017

SimonSapin commented May 18, 2017

SimonSapin commented May 18, 2017

bors-servo commented May 18, 2017

SimonSapin commented May 18, 2017

bors-servo commented May 18, 2017

bors-servo commented May 18, 2017

SimonSapin commented May 18, 2017

bors-servo commented May 18, 2017

bors-servo commented May 18, 2017

bors-servo commented May 19, 2017

servo / servo

Join GitHub today

Shrink selectors::Component, implement attr selector (in)case-sensitivity #16915

Shrink selectors::Component, implement attr selector (in)case-sensitivity #16915

Conversation

SimonSapin commented May 17, 2017 • edited

highfive commented May 17, 2017

highfive commented May 17, 2017

SimonSapin commented May 17, 2017

bors-servo commented May 17, 2017

bors-servo commented May 17, 2017

SimonSapin commented May 17, 2017

SimonSapin commented May 17, 2017

bors-servo commented May 17, 2017

SimonSapin commented May 17, 2017

bors-servo commented May 17, 2017

SimonSapin commented May 17, 2017

bors-servo commented May 17, 2017

bors-servo commented May 18, 2017

SimonSapin commented May 18, 2017

SimonSapin commented May 18, 2017

bors-servo commented May 18, 2017

SimonSapin commented May 18, 2017

bors-servo commented May 18, 2017

bors-servo commented May 18, 2017

SimonSapin commented May 18, 2017

bors-servo commented May 18, 2017

bors-servo commented May 18, 2017

bors-servo commented May 19, 2017

SimonSapin commented May 17, 2017 •

edited