When using a configuration without a swift compiler, we need to no-op
the swift annotations. Other, cleverer solutions beyond the has include
all fell flat in the face of the clang modules implementation used by
swift to parse-once use-everywhere each module.
The PageClient for SVGDecodedImageData needs to actually have the
definition of PageClient available in order to inherit from it. This
fixes importing this header from Objective-C or Swift.
The identifier "Protocol" is claimed by Objective-C and Swift for use
by the language's built-in protocol conformance feature, which is
similar to Rust traits or Java interfaces.
Rename LibProtocol -> LibRequests, and its namespace from Protocol to
Requests to accomodate this.
The values of attribute selectors are now compared case insensitively
by default if the attribute's document is not a HTML document, or the
element is not in the HTML namespace.
The `ThreadPoolLooper` should increment `m_busy_count` before attempting
to access the global queue. Otherwise, there exists a possible race
condition where `wait_for_all` checks the exit conditions before the
looper increments `m_busy_count` but after it empties the `ThreadPool`
queue.
Next, incrementing / decrementing `m_busy_count` is moved to be the
responsibility of `ThreadPoolLooper`. Otherwise, it is possible that
decrementing `m_busy_count` in the caller of `Looper::next` causes
`m_busy_count` to underflow if the call to `Looper::next` returns
before incrementing `m_busy_count`.
When adding tests for `ThreadPool` a handful of deadlocks can be
observed when worker threads wait on `m_work_available`.
The first deadlock is in the destruction of `ThreadPool` where it
is possible for a worker thread to be in the process of acquiring
`m_mutex` when the broadcast to `m_work_available` in the
destructor happens. This causes the destructor to hang on joining the
thread which is now perpetually waiting on `m_work_available`. This is
solved by repeatedly broadcasting on `m_work_available` until the thread
to join exits.
The second deadlock occurs when the final signal to `m_work_done` is
missed by the wait in `wait_for_all`. At this point all workers are in
the hot loop of attempting to get work from the work queue, however
since there is no work remaining all workers end up waiting on
`m_work_available`. At this point the `wait_for_all` call is also
waiting on `m_work_done`, which will never be signalled again as all
workers are waiting on `m_work_available`.
This requires 2 changes to fix, the first is that workers will signal
`m_done_work` before waiting on `m_work_available`. The second change is
to acquire `m_mutex` before checking the wait conditions as done when
using `wait_while`.
This allows us to use HeapFunction all of the way down, allowing us
to remove the Handle usage in after_session_callback for
create_new_child_navigable.
...For the completion steps. This is quite nice, as we can simply
capture this in the heap function where it is used instead of
needing to establish a new root.
Note that with these changes, to represent 'an empty algorithm', we now
use a null HeapFunction and do not invoke the steps.
This brings back the old behaviour of Value::to<short>() (and other
similar calls), which WASI depends on.
To make sure all similar issues are caught in the future, this commit
also introduces an static assertion in Value::to().
The Initialize* AOs for Intl formatters were removed some time ago, and
the formatter construction steps are now inlined in the constructors
themselves. InitializeNumberFormat was the one remaining initializer we
still had laying around.
This constructor has undergone a handful of editorial changes that we
fell behind on. But we weren't able to take the updates until now due to
a spec bug in those updates. See:
https://github.com/tc39/ecma402/commit/3f029b0
The result is that we can remove the inheritance of Intl::DateTimeFormat
from Unicode::DateTimeFormat; the former now contains the latter as an
internal slot.
We previously had 4 single-instance StyleValues for these keywords.
CSS-Typed-OM expects them keywords to be exposed as CSSKeywordValue, so
it's simpler to treat them the same. The single-instance behaviour is
kept by having StyleValue::create() use a cached instance for each of
these.
For a long time, we've used two terms, inconsistently:
- "Identifier" is a spec term, but refers to a sequence of alphanumeric
characters, which may or may not be a keyword. (Keywords are a
subset of all identifiers.)
- "ValueID" is entirely non-spec, and is directly called a "keyword" in
the CSS specs.
So to avoid confusion as much as possible, let's align with the spec
terminology. I've attempted to change variable names as well, but
obviously we use Keywords in a lot of places in LibWeb and so I may
have missed some.
One exception is that I've not renamed "valid-identifiers" in
Properties.json... I'd like to combine that and the "valid-types" array
together eventually, so there's no benefit to doing an extra rename
now.
Being constrained by this macro is going to make it more difficult to
refactor our StyleValue types to match the Typed-OM spec, so let's
reintroduce the boilerplate for now.
No functional changes.
The steps have been updated to indicate a few options that should be
defaulted based on locale preferences. Those steps have not yet been
implemented in this patch.
We were never able to implement anything other than a basic, locale-
unaware collator with the JSON export of the CLDR as it did not have
collation data. We can now use ICU to implement collation.
A display list should not contain coordinates shifted by scroll offset.
Instead, "scroll frame id" needs to be used. In the future it's going to
allow us reuse a display list in cases when only scroll offsets need to
be updated.
A display list should not contain coordinates shifted by scroll offset.
Instead, "scroll frame id" needs to be used. In the future it's going to
allow us reuse a display list in cases when only scroll offsets need to
be updated.
Removes code that accounts for clip frame's own overflow clip, because
it happens anyway on a first iteration of a loop through containing
blocks chain.
This change causes HTTP status codes to be set on cached HTTP responses.
Otherwise, without this change, no status codes at all are set on cached
HTTP responses — which causes all cached responses to default to being
loaded/served with a 200 status code. And as a result of that, if the
cached response is from a 30x redirect, then without this change, when
that cached 30x response is loaded, we don’t follow the redirect —
because we see a 200 status, rather than the expected/original 30x.
Fixes https://github.com/LadybirdBrowser/ladybird/issues/863
Note that this change also reverts the temporary workaround added in
https://github.com/LadybirdBrowser/ladybird/commit/f735c464d3f
(https://github.com/LadybirdBrowser/ladybird/pull/899).
If a HTTP 401 response we get does not contain a `WWW-Authenticate`
header, we should not trigger the logic to ask the user for credentials
and retry the request.
This part is hinted at in a TODO / 'Needs testing' remark in the spec
but needs to be fleshed out. Raised an upstream issue to do so:
https://github.com/whatwg/fetch/issues/1766
This fixes login forms triggering an infinite fetch loop when providing
incorrect credentials.
Co-Authored-By: Victor Tran <vicr12345@gmail.com>
The `start` and `end` value set the text alignment based on the computed
value of `direction`. The default value of `text-align` is now `start`
instead of `left`.
If given, the spec expects the input URL to be manipulated on the fly
as it is being parsed, and may ignore any errors thrown by the URL
parser.
Previously, we were not exactly following the specs assumption here
which resulted in us needed to make awkward copies of the URL in these
situations.
For most cases this is not an issue. But it does cause problems for
situations where URL parsing would result in a failure (which is
ignored by the caller), and the URL is _partially_ updated
while parsing.
Such a situation can occur when setting the host of an href alongside a
port number which is not valid. It is expected that this situation will
result in the host being updates - but not the port number.
Adjust the URL parser API so that it mutates the URL given (if any), and
adjust the callers accordingly.
Fixes two tests on https://wpt.live/url/url-setters-a-area.window.html
Currently, the following test case will actually copy both `a` and `b`
when the test macro is expanded:
ByteBuffer a = { some large buffer };
ByteBuffer b = { some other buffer };
EXPECT_EQ(a, b);
This patch redefines the expectation macros to avoid copying.
Because the type returned by to_string is a String, _not_ an
Optional<String>, the following code:
if (serialized_query.is_empty())
serialized_query = {};
Was achieving nothing at all! Make sure that the type is an
Optional<String> so that we're not just setting an empty string to an
empty string.
Quick sort is not a stable sort. This meant we had a subtle issue
implementing this portion of the spec comment:
> The relative order between name-value pairs with equal names must
> be preserved.
Switch to insertion sort which is a stable sort, and properly handles
keys which are the same.
Fixes 8 tests on https://wpt.live/url/urlsearchparams-sort.any.html
Before this change AddClipRect was a "special" because it didn't respect
scroll frame offset and was meant to be recorded using viewport-relative
coordinates. The motivation behind this was to record a "final" clip
rectangle computed by intersecting all clip rectangles used by a clip
frame. The disadvantage of this approach is that it blocks us from
implementing an optimisation to reuse display list if the only change is
in the scroll offset, because any scroll offset change leads to
invalidating all AddClipRect items within a list.
This change aligns AddClipRect with the rest of display list items by
making it account for scroll frame offset. It required discontinuing
the recording of the intersection of all clip rectangles within a clip
frame and instead producing an AddClipRect for each of them.
A nice side effect is the removal of code that shifts clip rectangle by
`enclosing_scroll_offset()` in a bunch of places, because now it happens
automatically in `DisplayList::apply_scroll_offsets()`.
This change causes the viewport to be treated as a "scroll frame,"
similar to how it already works for boxes with "overflow: scroll."
This means that, instead of encoding the viewport translation into a
display list, the items will be assigned the scroll frame id of the
viewport and then shifted by the scroll offset before execution. In the
future it will allow us to reuse a display list for repainting if only
scroll offset has changed.
As a side effect, it also removes the need for special handling of
"position: fixed" because compensating for the viewport offset while
painting or hit-testing is no longer necessary. Instead, anything
contained within a "position: fixed" element is simply not assigned
a scroll frame id, which means it is not shifted by the scroll offset.
This fixes a crash using URLSearchParams when provided a percent encoded
string which does not percent decode to valid UTF-8.
Fixes a crash running https://wpt.live/url/urlencoded-parser.any.html
There are (currently) no spec-tests ensuring that section ordering is
enforced, but it _is_ a part of the spec. A pull request to add this to
the specification testsuite has been opened at WebAssembly/spec#1775.
While introducing clip and scroll frame trees, I made a mistake by
assuming that the paintable tree includes boxes from nested navigables.
Therefore, this comment in the code was incorrect, and clip/scroll
frames were simply not assigned for iframes:
// NOTE: We only need to refresh the scroll state for traversables
// because they are responsible for tracking the state of all
// nested navigables.
As a result, anything with "overflow: scroll" is currently not
scrollable inside an iframe
This change fixes that by ensuring clip and scroll frames are assigned
and refreshed for each navigable. To achieve this, I had to modify the
display list building process to record a separate display list for each
navigable. This is necessary because scroll frame ids are local to a
navigable, making it impossible to call
`DisplayList::apply_scroll_offsets()` on a display list that contains
ids from multiple navigables.
In calculating the base size of a flex item, we have a piece of ad-hoc
code that deals with an item that does have an instrinsic aspect ratio,
but not a cross size to resolve that ratio against. In determining the
actual flex item size however, we also take into account the minimum
content width and height, which assumes the box' intrinsic width or
height when available. This would break having an image as a flex item,
which gets stretched to its maximum size within the flex container
instead of the flex item being shrunk to the instrinsic size of the
image.
Fix this by only stretching flex items that do not have an instrinsic
width nor height set.
Implements the corresponding encoders, selects the appropriate one when
encoding URL search params. If an encoder for the given encoding could
not be found, fallback to utf-8.
`m_needs_repaint = true` is not enough because it doesn't schedule
repaint of a parent navigable.
Fixes the bug when an iframe is not repainted after scrolling.
When converting a `Gfx::Bitmap` to a Skia bitmap, we cannot assume the
color data is unpremultiplied. For example, everything canvas-related
uses premultiplied color data:
https://html.spec.whatwg.org/multipage/canvas.html#premultiplied-alpha-and-the-2d-rendering-context
We were probably assuming unpremultiplied since that is what the PNG
decoder gives us. Since we now make `Gfx::Bitmap` identify what alpha
type is being used, we can instruct Skia a bit better :^)
Update our `EdgeFlagPathRasterizer` to use premultiplied alpha instead
of unpremultiplied so we can apply alpha correctly for path masks.
This fixes the dark borders sometimes visible when SVGs are blended
with a colored background.
This also exposed an issue with our `CanvasRenderingContext2D`, which is
supposed to hold a bitmap with premultiplied alpha internally but expose
a bitmap with unpremultiplied alpha in `CanvasImageData`. Expand our C2D
test to include the alpha channel as well.
Finally, this also exposed an off-by-one issue in
`EdgeFlagPathRasterizer` which caused the last scanlines for edges to
render incorrectly. We had some reference images which included these
corruptions (they were almost unnoticeable), so update them as well.
We use instances of `Gfx::Bitmap` to move pixel data all the way from
raw image bytes up to the Skia renderer. A vital piece of information
for correct blending of bitmaps is the alpha type, i.e. are we dealing
with premultiplied or unpremultiplied color values?
Premultiplied means that the RGB colors have been multiplied with the
associated alpha value, i.e. RGB(255, 255, 255) with an alpha of 2% is
stored as RGBA(5, 5, 5, 2%).
Unpremultiplied means that the original RGB colors are stored,
regardless of the alpha value. I.e. RGB(255, 255, 255) with an alpha of
2% is stored as RGBA(255, 255, 255, 2%).
It is important to know how the color data is stored in a
`Gfx::Bitmap`, because correct blending depends on knowing the alpha
type: premultiplied blending uses `S + (1 - A) * D`, while
unpremultiplied blending uses `A * S + (1 - A) * D`.
This adds the alpha type information to `Gfx::Bitmap` across the board.
It isn't used anywhere yet.
Knowing whether pixel color data is premultiplied or unpremultiplied is
useful in more places; factor it out so we can use the enum elsewhere.
Implement support for premultiplied alpha in `Color::with_alpha()` and
`::set_alpha()`.
If a DOM node is an element with pseudo-elements, but it has no child
DOM nodes and is not a shadow host, then the code that serializes its
pseudo-elements would get skipped, making them not show up in the
inspector.
Right now, we deviate from the CSSOM spec regarding our
CSSStyleDeclaration classes, so this is not as close to the spec as I'd
like. But it works, which means we'll be able to test pseudo-element
styling a lot more easily. :^)
As noted, this is hacky because the parser wasn't written to allow
parsing an individual component of a selector. (Fox example, the
convenient-sounding `parse_pseudo_simple_selector()` assumes the first
colon has already been consumed...) So until that changes, this parses
the input as an entire selector-list, and then throws it away if it's
not a single pseudo-element selector.
It's only temporary though, I promise. 😅
The script pulls in a dependency on the `yaml` python package. Instead
of updating all the docs and CI jobs to account for this, let's guard
calling the script behind our experimental flag instead.
Parsing last as an IPV4 number was not returning true in "ends with a
number" as the parsing of that part was overflowing. This means that the
URL is not considered to be an IPv4 address, and is treated as a valid
domain.
Helpfully, the spec also points out in a note that this step is
equivalent to simply checking that the last part ends with 0x followed
by only hex digits - which doesn't suffer from any overflow problem!
Arguably this is an editorial issue in the spec where this should be
clarified a little bit. But for now, fixing this fixes 3 sub tests in
WPT for:
https://wpt.live/url/url-constructor.any.html
Our heuristic was a bit too simplistic and would not run through the
ToASCII unicode algorithm which performs some extra validation. This
would cause invalid URLs that should fail to be parsed be mistakenly
accepted.
This fixes 8 tests in: https://wpt.live/url/url-constructor.any.html
We can't simply use the base URL as it may need to be modified in some
form. For example - for the included test, the fragment was previously
being included in the resulting URL.
This fixes 1 test on https://wpt.live/url/url-constructor.any.html
Instead of carrying the display list for a mask in each command that
might potentially be affected by "background-clip: text", this change
introduces a new AddMask command that is applied once for all
background layers within one box.
The new AddMask command includes a rectangle for the mask destination
that is translated by the corresponding scroll offset.
Fixes https://github.com/LadybirdBrowser/ladybird/issues/857
The spec didn't match how other browsers behave, and we dutifully did
what the spec said. A spec bug has been filed, so let's fix this locally
for now with a FIXME.
add_clip_rect() accepts a rectangle in viewport-relative coordinates,
so it must be translated by the enclosing scroll offset to be displayed
correctly inside a scrollable box.
This change fixes layering violation by moving to_px() calls to happen
before display list recording. Also it should make display list
recording a bit faster by resolving background properties beforehand.
This was a goof in cc55732332 which
resulted in the URL path getting double percent encoded. Since the path
already comes out percent encoded following the rules in the URL spec -
we don't need to percent encode again.
Fixes: #978
There were some extra steps in there which produced wrong results for
relative file URLs.
Fixes 7 test cases in: https://wpt.live/url/url-constructor.any.html
We also need to adjust the test results in TestURL. The behaviour tested
does not match how URL is specified to work as an absolute relative is
given.
The check for:
```
if (start_index >= end_index)
return {};
```
To prevent an out of bounds when trimming the start and end of the input
of whitespace was preventing valid URLs (only having whitespace in the
input) from being parsed.
Instead, prevent start_index from ever getting above end_index in the
first place, and don't treat empty inputs as an error.
Fixes one WPT test on:
https://wpt.live/url/url-constructor.any.html
This doesn't seem like something we will neccessarily do in the URL
class anyway due to performance reasons - unless strictly needed (like
for the DOMURL implementation).
I previously believed there was no way a detached document should have
layout information, but it turns out there is a way: getComputedStyle().
So we need to account for cases where we have a layout node, but no
navigable, since that is a state we can get into at this moment.
Fixes#354
For the SVG <use> element, we want to support loading HTML documents
that have a SVG element inside of it pointed to by the URL fragment.
In this situation we would need to fetch and parse the entire document
in SharedImageRequest (so that we can still cache the SVGs). Rename
SharedImageRequest to SharedResourceRequest to make the class a little
more generic for future usecases.
This closer mirrors handle_failed_fetch, making the handling slightly
more clear to understand. It also means that we don't need to take a
strong reference to this on a successful SVG resource load.
...When loading images through SharedImageRequest.
There is a small behavioural difference here - handle_failed_fetch
clears the pending callbacks whereas handled_failed_decode was
previously not. This does not seem intentional, and appears like a bug.
Implementing it this way is _slightly_ simpler - and also means we
don't need to take a strong handle to this in the case of loading an
SVG image.
