Stowage/ladybird
2
0
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2025-12-07 21:59:54 +00:00
Code Issues Projects Releases Packages Wiki Activity

LibWeb: Convert Ladybird notes in spec steps to // NB: ...

Browse source 
We have a couple of ways to designate spec notes and (our) developer
notes in comments, but we never really settled on a single approach. As
a result, we have a bit of a mixed bag of note comments on our hands.

To the extent that I could find them, I changed developer notes to
`// NB: ...` and changed spec notes to `// NOTE: ...`. The rationale for
this is that in most web specs, notes are prefixed by `NOTE: ...` so
this makes it easier to copy paste verbatim. The choice for `NB: ...` is
pretty arbitrary, but it makes it stand out from the regular spec notes
and it was already in wide use in our codebase.
This commit is contained in:
Jelle Raaijmakers 2025-11-17 13:10:41 +01:00 • committed by Tim Flynn
parent 9394c9a10b
commit 41eb7251e4
Notes: github-actions[bot] 2025-11-18 14:09:02 +00:00
11 changed files with 52 additions and 44 deletions
Download patch file Download diff file Expand all files Collapse all files

12
Libraries/LibWeb/Bindings/MainThreadVM.cpp
View file

@ -535,12 +535,12 @@ void initialize_main_thread_vm(AgentType type)
return;
}
// Spec-Note: This step is essentially validating all of the requested module specifiers and type attributes
// when the first call to HostLoadImportedModule for a static module dependency list is made, to
// avoid further loading operations in the case any one of the dependencies has a static error.
// We treat a module with unresolvable module specifiers or unsupported type attributes the same
// as one that cannot be parsed; in both cases, a syntactic issue makes it impossible to ever
// contemplate linking the module later.
// NOTE: This step is essentially validating all of the requested module specifiers and type attributes
// when the first call to HostLoadImportedModule for a static module dependency list is made, to
// avoid further loading operations in the case any one of the dependencies has a static error. We
// treat a module with unresolvable module specifiers or unsupported type attributes the same as
// one that cannot be parsed; in both cases, a syntactic issue makes it impossible to ever
// contemplate linking the module later.
}
}

12
Libraries/LibWeb/DOM/EventTarget.cpp
View file

@ -734,16 +734,18 @@ JS::ThrowCompletionOr<void> EventTarget::process_event_handler_for_event(FlyStri
if (special_error_event_handling) {
// -> If special error event handling is true
// If return value is true, then set event's canceled flag.
// NOTE: the return type of EventHandler is `any`, so no coercion happens, meaning we have to check if it's a boolean first.
// NB: the return type of EventHandler is `any`, so no coercion happens, meaning we have to check if it's a
// boolean first.
if (return_value.is_boolean() && return_value.as_bool())
event.set_cancelled(true);
} else {
// -> Otherwise
// If return value is false, then set event's canceled flag.
// NOTE: the return type of EventHandler is `any`, so no coercion happens, meaning we have to check if it's a boolean first.
// Spec-Note: If we've gotten to this "Otherwise" clause because event's type is "beforeunload" but event is
// not a BeforeUnloadEvent object, then return value will never be false, since in such cases
// return value will have been coerced into either null or a DOMString.
// NB: the return type of EventHandler is `any`, so no coercion happens, meaning we have to check if it's a
// boolean first.
// NOTE: If we've gotten to this "Otherwise" clause because event's type is "beforeunload" but event is not a
// BeforeUnloadEvent object, then return value will never be false, since in such cases return value will
// have been coerced into either null or a DOMString.
if (return_value.is_boolean() && !return_value.as_bool() && !is_beforeunload)
event.set_cancelled(true);
}

8
Libraries/LibWeb/DOM/Node.cpp
View file

@ -786,10 +786,10 @@ void Node::insert_before(GC::Ref<Node> node, GC::Ptr<Node> child, bool suppress_
children_changed(&metadata);
// 10. Let staticNodeList be a list of nodes, initially « ».
// Spec-Note: We collect all nodes before calling the post-connection steps on any one of them, instead of calling
// the post-connection steps while we’re traversing the node tree. This is because the post-connection
// steps can modify the tree’s structure, making live traversal unsafe, possibly leading to the
// post-connection steps being called multiple times on the same node.
// NOTE: We collect all nodes before calling the post-connection steps on any one of them, instead of calling the
// post-connection steps while we’re traversing the node tree. This is because the post-connection steps can
// modify the tree’s structure, making live traversal unsafe, possibly leading to the post-connection steps
// being called multiple times on the same node.
GC::RootVector<GC::Ref<Node>> static_node_list(heap());
// 11. For each node of nodes, in tree order:

6
Libraries/LibWeb/FileAPI/FileReader.cpp
View file

@ -258,7 +258,8 @@ WebIDL::ExceptionOr<void> FileReader::read_operation(Blob& blob, Type type, Opti
if (m_state != State::Loading)
dispatch_event(DOM::Event::create(realm, HTML::EventNames::loadend));
// Spec-Note: Event handler for the load or error events could have started another load, if that happens the loadend event for this load is not fired.
// NOTE: Event handler for the load or error events could have started another load, if that happens
// the loadend event for this load is not fired.
}));
return;
@ -278,7 +279,8 @@ WebIDL::ExceptionOr<void> FileReader::read_operation(Blob& blob, Type type, Opti
if (m_state != State::Loading)
dispatch_event(DOM::Event::create(realm, HTML::EventNames::loadend));
// Spec-Note: Event handler for the error event could have started another load, if that happens the loadend event for this load is not fired.
// NOTE: Event handler for the error event could have started another load, if that happens the
// loadend event for this load is not fired.
}));
return;

10
Libraries/LibWeb/HTML/HTMLButtonElement.cpp
View file

@ -154,9 +154,9 @@ bool HTMLButtonElement::has_activation_behavior() const
return true;
}
// https://html.spec.whatwg.org/multipage/form-elements.html#the-button-element:activation-behaviour
void HTMLButtonElement::activation_behavior(DOM::Event const& event)
{
// https://html.spec.whatwg.org/multipage/form-elements.html#the-button-element:activation-behaviour
// 1. If element is disabled, then return.
if (!enabled())
return;
@ -223,8 +223,12 @@ void HTMLButtonElement::activation_behavior(DOM::Event const& event)
return;
}
// 5. Let continue be the result of firing an event named command at target, using CommandEvent, with its command attribute initialized to command, its source attribute initialized to element, and its cancelable and composed attributes initialized to true.
// SPEC-NOTE: DOM standard issue #1328 tracks how to better standardize associated event data in a way which makes sense on Events. Currently an event attribute initialized to a value cannot also have a getter, and so an internal slot (or map of additional fields) is required to properly specify this.
// 5. Let continue be the result of firing an event named command at target, using CommandEvent, with its
// command attribute initialized to command, its source attribute initialized to element, and its cancelable
// and composed attributes initialized to true.
// NOTE: DOM standard issue #1328 tracks how to better standardize associated event data in a way which makes
// sense on Events. Currently an event attribute initialized to a value cannot also have a getter, and so
// an internal slot (or map of additional fields) is required to properly specify this.
CommandEventInit event_init {};
event_init.command = command;
event_init.source = this;

16
Libraries/LibWeb/HTML/Navigation.cpp
View file

@ -244,10 +244,10 @@ WebIDL::ExceptionOr<NavigationResult> Navigation::navigate(String url, Navigatio
// 4. Let state be options["state"], if it exists; otherwise, undefined.
auto state = options.state.value_or(JS::js_undefined());
// 5. Let serializedState be StructuredSerializeForStorage(state).
// If this throws an exception, then return an early error result for that exception.
// Spec-Note: It is important to perform this step early, since serialization can invoke web developer code,
// which in turn might change various things we check in later steps.
// 5. Let serializedState be StructuredSerializeForStorage(state). If this throws an exception, then return an early
// error result for that exception.
// NOTE: It is important to perform this step early, since serialization can invoke web developer code, which in
// turn might change various things we check in later steps.
auto serialized_state_or_error = structured_serialize_for_storage(vm, state);
if (serialized_state_or_error.is_error()) {
return early_error_result(serialized_state_or_error.release_error());
@ -307,10 +307,10 @@ WebIDL::ExceptionOr<NavigationResult> Navigation::reload(NavigationReloadOptions
// 2. Let serializedState be StructuredSerializeForStorage(undefined).
auto serialized_state = MUST(structured_serialize_for_storage(vm, JS::js_undefined()));
// 3. If options["state"] exists, then set serializedState to StructuredSerializeForStorage(options["state"]).
// If this throws an exception, then return an early error result for that exception.
// Spec-Note: It is important to perform this step early, since serialization can invoke web developer
// code, which in turn might change various things we check in later steps.
// 3. If options["state"] exists, then set serializedState to StructuredSerializeForStorage(options["state"]). If
// this throws an exception, then return an early error result for that exception.
// NOTE: It is important to perform this step early, since serialization can invoke web developer code, which in
// turn might change various things we check in later steps.
if (options.state.has_value()) {
auto serialized_state_or_error = structured_serialize_for_storage(vm, options.state.value());
if (serialized_state_or_error.is_error())

3
Libraries/LibWeb/HTML/Scripting/Fetching.cpp
View file

@ -157,7 +157,8 @@ WebIDL::ExceptionOr<URL::URL> resolve_module_specifier(Optional<Script&> referri
result = TRY(resolve_imports_match(normalized_specifier.to_byte_string(), as_url, import_map.imports()));
// 12. If result is null, set it to asURL.
// Spec-Note: By this point, if result was null, specifier wasn't remapped to anything by importMap, but it might have been able to be turned into a URL.
// NOTE: By this point, if result was null, specifier wasn't remapped to anything by importMap, but it might have
// been able to be turned into a URL.
if (!result.has_value())
result = as_url;

4
Libraries/LibWeb/HTML/Scripting/ImportMap.cpp
View file

@ -300,8 +300,8 @@ void merge_existing_and_new_import_maps(Window& global, ImportMap& new_import_ma
// 1. Let newImportMapScopes be a deep copy of newImportMap's scopes.
auto new_import_map_scopes = new_import_map.scopes();
// Spec-Note: We're mutating these copies and removing items from them when they are used to ignore scope-specific
// rules. This is true for newImportMapScopes, as well as to newImportMapImports below.
// NOTE: We're mutating these copies and removing items from them when they are used to ignore scope-specific rules.
// This is true for newImportMapScopes, as well as to newImportMapImports below.
// 2. Let oldImportMap be global's import map.
auto& old_import_map = global.import_map();

12
Libraries/LibWeb/HTML/Window.h
View file

@ -58,8 +58,8 @@ struct SpecifierResolution {
// A specifier as a URL
// A URL-or-null that represents the URL in case of a URL-like module specifier.
//
// Spec-Note: Implementations can replace specifier as a URL with a boolean that indicates
// that the specifier is either bare or URL-like that is special.
// NOTE: Implementations can replace specifier as a URL with a boolean that indicates that the specifier is either
// bare or URL-like that is special.
bool specifier_is_null_or_url_like_that_is_special { false };
};
@ -300,10 +300,10 @@ private:
// https://html.spec.whatwg.org/multipage/webappapis.html#resolved-module-set
// A global object has a resolved module set, a set of specifier resolution records, initially empty.
//
// Spec-Note: The resolved module set ensures that module specifier resolution returns the same result when called
// multiple times with the same (referrer, specifier) pair. It does that by ensuring that import map rules
// that impact the specifier in its referrer's scope cannot be defined after its initial resolution. For
// now, only Window global objects have their module set data structures modified from the initial empty one.
// NOTE: The resolved module set ensures that module specifier resolution returns the same result when called
// multiple times with the same (referrer, specifier) pair. It does that by ensuring that import map rules
// that impact the specifier in its referrer's scope cannot be defined after its initial resolution. For now,
// only Window global objects have their module set data structures modified from the initial empty one.
Vector<SpecifierResolution> m_resolved_module_set;
GC::Ptr<CSS::Screen> m_screen;

8
Libraries/LibWeb/SVG/SVGAnimatedNumber.cpp
View file

@ -63,8 +63,8 @@ void SVGAnimatedNumber::set_base_val(float new_value)
// 3. Let second be the second number in current if it has been explicitly specified, and if not, the implicit
// value as described in the definition of the attribute.
// LB-NOTE: All known usages of <number-optional-number> specify that a missing second number defaults to the
// value of the first number.
// NB: All known usages of <number-optional-number> specify that a missing second number defaults to the value
// of the first number.
auto second = current_values.size() > 1 && !current_values[1].is_empty()
? parse_value_or_initial(current_values[1])
: first;
@ -130,8 +130,8 @@ float SVGAnimatedNumber::get_base_or_anim_value() const
// 2. Otherwise, this SVGAnimatedNumber object reflects the second number. Return the second value in value if
// it has been explicitly specified, and if not, return the implicit value as described in the definition of
// the attribute.
// LB-NOTE: All known usages of <number-optional-number> specify that a missing second number defaults to the
// value of the first number.
// NB: All known usages of <number-optional-number> specify that a missing second number defaults to the value
// of the first number.
VERIFY(m_value_represented == ValueRepresented::Second);
if (values.size() > 1 && !values[1].is_empty())
return parse_value_or_initial(values[1]);

5
Libraries/LibWeb/ServiceWorker/Job.cpp
View file

@ -499,9 +499,8 @@ static void run_job(JS::VM& vm, JobQueue& job_queue)
});
// FIXME: How does the user agent ensure this happens? Is this a normative note?
// Spec-Note:
// For a register job and an update job, the user agent delays queuing a task for running the job
// until after a DOMContentLoaded event has been dispatched to the document that initiated the job.
// NOTE: For a register job and an update job, the user agent delays queuing a task for running the job until after
// a DOMContentLoaded event has been dispatched to the document that initiated the job.
// FIXME: Spec should be updated to avoid 'queue a task' and use 'queue a global task' instead
// FIXME: On which task source? On which event loop? On behalf of which document?