Bug 247858 - Use correct urls in PageSerializer for CSS imports
Summary: Use correct urls in PageSerializer for CSS imports
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-11-13 04:16 PST by Ahmad Saleem
Modified: 2023-02-08 21:15 PST (History)
6 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ahmad Saleem 2022-11-13 04:16:16 PST 
Hi Team,

While going through Blink Commits, I came across another commit, which we could merge, unfortunately I can't find the testcase to confirm whether this is an issue or not but if it is not, appreciate if this can be closed.

Blink Commit - https://chromium.googlesource.com/chromium/blink/+/76d430db064030c5c38bc68b21f4d079af2851c3

Webkit Commit - https://github.com/WebKit/WebKit/blob/5a9504aef82d929917a680a05a3fc17f7f6b4dde/Source/WebCore/page/PageSerializer.cpp#L252

I might not be able to do this because this does not have any test case and I don't know how to implement .CPP test files like Blink in Webkit.

Just raising it for someone else to confirm whether we still have this issue or it is something changed in the web-spec and we don't need this now.

Thanks!
Comment 1 Ahmad Saleem 2022-11-13 04:19:08 PST 
In Line 255, changing from:

URL importURL = document->completeURL(importRule.href());

to

URL importURL = URL(document->styleSheet.baseURL(), importRule.href());

or something else.
Comment 2 Radar WebKit Bug Importer 2022-11-20 04:17:13 PST 
<rdar://problem/102559387>
Comment 3 Ahmad Saleem 2023-02-08 04:38:25 PST 
ASSERT(styleSheet->baseURL().isValid());
URL importURL = URL(styleSheet->baseURL(), importRule.href());

^ This will compile this based on the patch but I looked into WPT whether it will have any passes but it seems there is none. :-(

Do we want to merge this?
Comment 4 Ryosuke Niwa 2023-02-08 21:15:13 PST 
It's probably still relevant but we do need to add a test case for sure.