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

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.

Author: gmta

Libraries/LibWeb/Bindings/MainThreadVM.cpp

@@ -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.
             }
         }
 

Libraries/LibWeb/DOM/EventTarget.cpp

@@ -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);
     }

Libraries/LibWeb/DOM/Node.cpp

@@ -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:

Libraries/LibWeb/FileAPI/FileReader.cpp

@@ -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;

Libraries/LibWeb/HTML/HTMLButtonElement.cpp

@@ -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;

Libraries/LibWeb/HTML/Navigation.cpp

@@ -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())

Libraries/LibWeb/HTML/Scripting/Fetching.cpp

@@ -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;
 

Libraries/LibWeb/HTML/Scripting/ImportMap.cpp

@@ -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();

Libraries/LibWeb/HTML/Window.h

@@ -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 @@ class WEB_API Window final
     // 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;

Libraries/LibWeb/SVG/SVGAnimatedNumber.cpp

@@ -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]);