Crash in [@ core::option::expect_none_failed | geckoservo::glue::Servo_DeclarationBlock_GetCssText]
Categories
(Core :: CSS Parsing and Computation, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr78 | --- | unaffected |
firefox88 | --- | unaffected |
firefox89 | --- | unaffected |
firefox90 | blocking | fixed |
People
(Reporter: emilio, Assigned: emilio)
References
(Regression)
Details
(Keywords: crash, regression)
Crash Data
Attachments
(2 files)
Crash report: https://crash-stats.mozilla.org/report/index/9358fffe-d5a1-4524-98bd-e05e50210514
MOZ_CRASH Reason: called `Result::unwrap()` on an `Err` value: Error
Top 10 frames of crashing thread:
0 xul.dll RustMozCrash mozglue/static/rust/wrappers.cpp:16
1 xul.dll mozglue_static::panic_hook mozglue/static/rust/lib.rs:89
2 xul.dll core::ops::function::Fn::call<fn ../88f19c6dab716c6281af7602e30f413e809c5974/library/core/src/ops/function.rs:227
3 xul.dll std::panicking::rust_panic_with_hook ../88f19c6dab716c6281af7602e30f413e809c5974//library/std/src/panicking.rs:595
4 xul.dll std::panicking::begin_panic_handler::{{closure}} ../88f19c6dab716c6281af7602e30f413e809c5974//library/std/src/panicking.rs:497
5 xul.dll std::sys_common::backtrace::__rust_end_short_backtrace<closure-0, !> ../88f19c6dab716c6281af7602e30f413e809c5974//library/std/src/sys_common/backtrace.rs:141
6 xul.dll std::panicking::begin_panic_handler ../88f19c6dab716c6281af7602e30f413e809c5974//library/std/src/panicking.rs:493
7 xul.dll core::panicking::panic_fmt ../88f19c6dab716c6281af7602e30f413e809c5974//library/core/src/panicking.rs:92
8 xul.dll core::option::expect_none_failed ../88f19c6dab716c6281af7602e30f413e809c5974//library/core/src/option.rs:1329
9 xul.dll geckoservo::glue::Servo_DeclarationBlock_GetCssText servo/ports/geckolib/glue.rs:4357
Updated•3 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
All is special, and that'd cause an error later on.
Assignee | ||
Comment 2•3 years ago
|
||
They are not. The serialization code already checks if the result is
empty, which can happen for other reasons. This makes the code a bit
more resilient to misuse.
Assignee | ||
Comment 3•3 years ago
|
||
I'll land this unreviewed to stop the crash, since they're really simple, but Daniel / Boris, can you sanity-check when you get a chance? See the test-case in the first patch for what's going on.
Before, the bug was wallpapered because those properties were part of the all
shorthand but weren't returned by getComputedStyle
. Either of the two patches fix the bug. The former avoids doing a bit of extra work.
Assignee | ||
Comment 4•3 years ago
|
||
Err, see the comment above.
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ac590dee77f5 Avoid trying to serialize 'all' with stuff that isn't variable references or CSS wide keywords. https://hg.mozilla.org/integration/autoland/rev/72f541c3031f Avoid returning fmt::Errors for things that are not formatting errors.
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/28996 for changes under testing/web-platform/tests
Updated•3 years ago
|
Updated•3 years ago
|
Comment 7•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ac590dee77f5
https://hg.mozilla.org/mozilla-central/rev/72f541c3031f
Upstream PR merged by moz-wptsync-bot
Comment 9•3 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)
I'll land this unreviewed to stop the crash, since they're really simple, but Daniel / Boris, can you sanity-check when you get a chance? See the test-case in the first patch for what's going on.
Boris probably knows this css-internals code better than I do, so he may see something that I don't. But this seems fine to me. Thanks for jumping on this!
(I did notice one extremely-nitpicky typo -- "CSS wide" is supposed to be hyphenated -- which I left as an inline nit on part 1. If you feel like fixing that in a followup here, that might be nice, though also not a big deal.)
Updated•3 years ago
|
Updated•3 years ago
|
Comment 10•3 years ago
•
|
||
(In reply to Daniel Holbert [:dholbert] from comment #9)
(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)
I'll land this unreviewed to stop the crash, since they're really simple, but Daniel / Boris, can you sanity-check when you get a chance? See the test-case in the first patch for what's going on.
Boris probably knows this css-internals code better than I do, so he may see something that I don't. But this seems fine to me. Thanks for jumping on this!
Looks fine with me as well. Thanks for the quick look. (I'm fine with both patches.)
Updated•3 years ago
|
Description
•