Closed Bug 1384275 Opened 7 years ago Closed 7 years ago

stylo: icon fonts are not loaded on fluentcpp.com

Categories

(Core :: CSS Parsing and Computation, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: cpeterson, Assigned: heycam)

References

(Blocks 1 open bug, )

Details

Attachments

(3 files, 1 obsolete file)

STR:
1. Load http://www.fluentcpp.com/2017/07/25/understandable-statements-run-slower/
2. Look at the buttons below the text "Jonathan Boccara's blog" in page's left column.
3. Scroll down the page until a "Tweet" button is shown at the top of the page.

RESULT:
When Stylo is disabled, the buttons are rendered as images of a hamburger menu button, a magnifying glass, and the Twitter logo respectively. When Stylo is enabled, the buttons are rendered as unrecognized Unicode characters.
Compared to normal render, the custom web font was not loaded. Stylo was using default system font.

icomoon - woff

Used as: "voice-icomoon"

@font-face {
  font-family: "voice-icomoon";
  font-style: normal;
  font-weight: normal;
  src: url("//www.fluentcpp.com/wp-content/themes/voice/fonts/voice-icomoon.eot?#iefix4seflc") format("embedded-opentype"), url("//www.fluentcpp.com/wp-content/themes/voice/fonts/voice-icomoon.woff?4seflc") format("woff"), url("//www.fluentcpp.com/wp-content/themes/voice/fonts/voice-icomoon.ttf?4seflc") format("truetype"), url("//www.fluentcpp.com/wp-content/themes/voice/fonts/voice-icomoon.svg?4seflc#icomoon") format("svg");
}
Flags: needinfo?(cam)
See Also: → 1384270
When I use the CSSOM to inspect the rules from the style sheet that includes that @font-face rule, it doesn't appear.  The rules directly before it are some @import rules, incorrectly placed in the middle of a style sheet.  I wonder if this is an issue with the C++ rule wrappers?
Flags: needinfo?(cam)
When I dump the contents of the StylesheetContents object, the @import rules and the @font-face rules are missing.
Attached file reduced test
Assignee: nobody → cam
Status: NEW → ASSIGNED
Comment on attachment 8891606 [details]
Bug 1384275 - style: Don't remain in an invalid state when encountering an at-rule in the wrong place.

https://reviewboard.mozilla.org/r/162718/#review168072

::: servo/components/style/stylesheets/mod.rs:262
(Diff revision 1)
> -        match parse_one_rule(&mut input, &mut rule_parser) {
> -            Ok(result) => Ok((result, rule_parser.state)),
> -            Err(_) => {
> -                Err(match rule_parser.state {
> -                    State::Invalid => SingleRuleParseError::Hierarchy,
> -                    _ => SingleRuleParseError::Syntax,
> -                })
> +        parse_one_rule(&mut input, &mut rule_parser)
> +            .map(|result| (result, rule_parser.state))
> +            .map_err(|_| {
> +                if rule_parser.get_and_reset_had_hierarchy_error() {
> +                    SingleRuleParseError::Hierarchy
> +                } else {
> +                    SingleRuleParseError::Syntax
> -            }
> -        }
> +                }
> +            })

Hmm, this seemed to cause a test failure.  Will look into it.

```
 FAIL | layout/style/test/test_group_insertRule.html | insert_import_throws
insert_import_throws: assert_throws: inserting a disallowed rule should throw HIERARCHY_REQUEST_ERR function "function() {
                         grouping_rule.inser..." threw object "SyntaxError: An invalid or illegal string was specified" that is not a DOMException HIERARCHY_REQUEST_ERR: property "code" is equal to 12, expected 3
```
(I just forgot to add the actual assignments to had_hierarchy_error...)
Comment on attachment 8891606 [details]
Bug 1384275 - style: Don't remain in an invalid state when encountering an at-rule in the wrong place.

https://reviewboard.mozilla.org/r/162718/#review168078

Nice find!

::: servo/components/style/stylesheets/rule_parser.rs:83
(Diff revision 1)
> +
> +    /// Returns whether we previously tried to parse a rule that was invalid
> +    /// due to being in the wrong place (e.g. an @import rule was found after
> +    /// a regular style rule).  The state of this flag is reset when this
> +    /// function is called.
> +    pub fn get_and_reset_had_hierarchy_error(&mut self) -> bool {

nit: The rust naming for this would be something like `take_had_hierarchy_error`
Attachment #8891606 - Flags: review?(emilio+bugs) → review+
Comment on attachment 8891607 [details]
Bug 1384275 - Reftest.

https://reviewboard.mozilla.org/r/162720/#review168084

::: layout/reftests/bugs/1384275-1-ref.html:6
(Diff revision 2)
> +<!DOCTYPE html>
> +<style>
> +p { color: red; }
> +@supports (color: green) {
> +  p { color: green; }
> +}

nit: I'd just use `p { color: green }` instead of `@supports` in the ref, but your call.
Attachment #8891607 - Flags: review?(emilio+bugs) → review+
Attachment #8891606 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/6a990e8c471a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
(In reply to Chris Peterson [:cpeterson] from comment #0)
> STR:
> 1. Load
> http://www.fluentcpp.com/2017/07/25/understandable-statements-run-slower/
> 2. Look at the buttons below the text "Jonathan Boccara's blog" in page's
> left column.
> 3. Scroll down the page until a "Tweet" button is shown at the top of the
> page.
> 
> RESULT:
> When Stylo is disabled, the buttons are rendered as images of a hamburger
> menu button, a magnifying glass, and the Twitter logo respectively. When
> Stylo is enabled, the buttons are rendered as unrecognized Unicode
> characters.

Verified fixed in Nightly 56 x64 20170730100307 @ Debian Testing
(by attachment 8891606 [details] = https://github.com/servo/servo/pull/17918)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.