1341690 - stylo: background-image doesn't work in inline style

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

This is what causes the really-big-background.html test failure.  Probably relevant error message:

WARN:style::values::specified::url: stylo: skipping declaration without ParserContextExtraData

Comment 1

[Manish Goregaokar [:manishearth]]

This is getting fixed in bug 1329093

Comment 2

[Bobby Holley (:bholley)]

Can we reenable the testcase in comment 0 now?

Comment 3

[Manish Goregaokar [:manishearth]]

yes

Comment 4

[Manish Goregaokar [:manishearth]]

Attachment: Bug 1341690 - stylo: Make URLs work in inline style;

Review commit: https://reviewboard.mozilla.org/r/119296/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/119296/

Comment 5

[Manish Goregaokar [:manishearth]]

Hm, this test still fails. Wat.

Comment 6

[Manish Goregaokar [:manishearth]]

Ah, I'd missed ParseStyleAttribute in my other patch. Fixing.

Comment 7

[Manish Goregaokar [:manishearth]]

Comment on attachment 8846191 [details]
Bug 1341690 - stylo: Make URLs work in inline style;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/119296/diff/1-2/

Comment 8

[Manish Goregaokar [:manishearth]]

Comment on attachment 8846191 [details]
Bug 1341690 - stylo: Make URLs work in inline style;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/119296/diff/2-3/

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8846191 [details]
Bug 1341690 - stylo: Make URLs work in inline style;

https://reviewboard.mozilla.org/r/119296/#review121220

r=me with that.

::: layout/style/nsDOMCSSDeclaration.cpp:135
(Diff revision 3)
>    mozAutoDocConditionalContentUpdateBatch autoUpdate(DocToUpdate(), true);
>  
>    RefPtr<DeclarationBlock> newdecl;
>    if (olddecl->IsServo()) {
> -    newdecl = ServoDeclarationBlock::FromCssText(aCssText);
> +    GeckoParserExtraData data(env.mBaseURI, env.mSheetURI, env.mPrincipal);
> +    nsCString baseString;

Let's move this to `FromCSSText`, where we already have the base uri in `GeckoParserExtraData` instead of duplicating it twice.

Comment 10

[Manish Goregaokar [:manishearth]]

Comment on attachment 8846191 [details]
Bug 1341690 - stylo: Make URLs work in inline style;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/119296/diff/3-4/

Comment 11

[Manish Goregaokar [:manishearth]]

Servo at https://github.com/servo/servo/pull/15915

Comment 12

[Manish Goregaokar [:manishearth]]

Comment on attachment 8846191 [details]
Bug 1341690 - stylo: Make URLs work in inline style;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/119296/diff/4-5/

Comment 13

[Bobby Holley (:bholley)]

Looks like the test should be able to be reenabled now.

Comment 14

[Manish Goregaokar [:manishearth]]

Yep, just need to autoland this, but that's blocked on VCS sync waking up

Comment 15

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 15ad85dc898c -d e18199b999a3: rebasing 381624:15ad85dc898c "Bug 1341690 - stylo: Make URLs work in inline style; r=emilio" (tip)
merging dom/base/nsAttrValue.cpp
merging layout/style/ServoBindingList.h
merging layout/style/test/stylo-failures.md
warning: conflicts while merging layout/style/test/stylo-failures.md! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8949490f91a7
stylo: Make URLs work in inline style; r=emilio

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8949490f91a7