mirrors/ladybird
0
0
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2025-08-26 04:07:51 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 9

LibWeb: Improve list item marker positioning and alpha/roman text

Browse source 
This commit is a three-parter that is hard to separate without breaking
marker rendering:

  1. Any marker style that results in a string, except for a literal
     string (e.g. `list-style-type: "@"`), should get the string ". "
     appended. We forgot to do this for the alpha and roman types.

  2. Instead of using the "pixel size rounded up" from a font and adding
     an arbitrary 1 to that, we now use the exact pixel size for as long
     as possible to improve our vertical positioning of markers.

  3. Instead of always adding a "default marker width" to the marker
     content width, we now only do this if we did not have text metrics
     available (i.e. the marker style is not a text type). This greatly
     improves horizontal positioning of text markers.
This commit is contained in:
Jelle Raaijmakers 2025-07-15 13:37:13 +02:00 • committed by Sam Atkins
commit 788d5368a7
Notes: github-actions[bot] 2025-07-15 18:06:51 +00:00
26 changed files with 152 additions and 129 deletions
Download patch file Download diff file Expand all files Collapse all files

16
Libraries/LibWeb/Layout/BlockFormattingContext.cpp
View file

@ -1199,18 +1199,17 @@ void BlockFormattingContext::ensure_sizes_correct_for_left_offset_calculation(Li
image_height = list_style_image->natural_height().value_or(0);
}
auto default_marker_width = max(4, marker.first_available_font().pixel_size_rounded_up() - 4);
auto marker_text = marker.text().value_or({});
if (marker_text.is_empty()) {
auto marker_text = marker.text();
if (!marker_text.has_value()) {
auto default_marker_width = max(4, marker.first_available_font().pixel_size() - 4);
marker_state.set_content_width(image_width + default_marker_width);
} else {
// FIXME: Use per-code-point fonts to measure text.
auto text_width = marker.first_available_font().width(marker_text.code_points());
auto text_width = marker.first_available_font().width(marker_text.value().code_points());
marker_state.set_content_width(image_width + CSSPixels::nearest_value_for(text_width));
}
marker_state.set_content_height(max(image_height, marker.first_available_font().pixel_size_rounded_up() + 1));
marker_state.set_content_height(max(image_height, CSSPixels { marker.first_available_font().pixel_size() }));
}
void BlockFormattingContext::layout_list_item_marker(ListItemBox const& list_item_box, CSSPixels const& left_space_before_list_item_elements_formatted)
@ -1222,7 +1221,8 @@ void BlockFormattingContext::layout_list_item_marker(ListItemBox const& list_ite
auto& marker_state = m_state.get_mutable(marker);
auto& list_item_state = m_state.get_mutable(list_item_box);
auto default_marker_width = max(4, marker.first_available_font().pixel_size_rounded_up() - 4);
auto marker_text = marker.text();
auto default_marker_width = marker_text.has_value() ? 0 : max(4, marker.first_available_font().pixel_size() - 4);
auto final_marker_width = marker_state.content_width() + default_marker_width;
if (marker.list_style_position() == CSS::ListStylePosition::Inside) {
@ -1232,7 +1232,7 @@ void BlockFormattingContext::layout_list_item_marker(ListItemBox const& list_ite
auto offset_y = max(CSSPixels(0), (marker.computed_values().line_height() - marker_state.content_height()) / 2);
marker_state.set_content_offset({ left_space_before_list_item_elements_formatted - final_marker_width, offset_y });
marker_state.set_content_offset({ left_space_before_list_item_elements_formatted - final_marker_width, round(offset_y) });
if (marker_state.content_height() > list_item_state.content_height())
list_item_state.set_content_height(marker_state.content_height());

39
Libraries/LibWeb/Layout/ListItemMarkerBox.cpp
View file

@ -28,33 +28,40 @@ Optional<String> ListItemMarkerBox::text() const
return m_list_style_type.visit(
[index](CSS::CounterStyleNameKeyword keyword) -> Optional<String> {
String text;
switch (keyword) {
case CSS::CounterStyleNameKeyword::Square:
case CSS::CounterStyleNameKeyword::Circle:
case CSS::CounterStyleNameKeyword::Disc:
case CSS::CounterStyleNameKeyword::DisclosureClosed:
case CSS::CounterStyleNameKeyword::DisclosureOpen:
return {};
case CSS::CounterStyleNameKeyword::Decimal:
return MUST(String::formatted("{}.", index));
case CSS::CounterStyleNameKeyword::DecimalLeadingZero:
// This is weird, but in accordance to spec.
return MUST(index < 10 ? String::formatted("0{}.", index) : String::formatted("{}.", index));
case CSS::CounterStyleNameKeyword::LowerAlpha:
case CSS::CounterStyleNameKeyword::LowerLatin:
return String::bijective_base_from(index - 1, String::Case::Lower);
case CSS::CounterStyleNameKeyword::UpperAlpha:
case CSS::CounterStyleNameKeyword::UpperLatin:
return String::bijective_base_from(index - 1, String::Case::Upper);
case CSS::CounterStyleNameKeyword::LowerRoman:
return String::roman_number_from(index, String::Case::Lower);
case CSS::CounterStyleNameKeyword::UpperRoman:
return String::roman_number_from(index, String::Case::Upper);
case CSS::CounterStyleNameKeyword::None:
return {};
case CSS::CounterStyleNameKeyword::Decimal:
text = String::number(index);
break;
case CSS::CounterStyleNameKeyword::DecimalLeadingZero:
// This is weird, but in accordance to spec.
text = index < 10 ? MUST(String::formatted("0{}", index)) : String::number(index);
break;
case CSS::CounterStyleNameKeyword::LowerAlpha:
case CSS::CounterStyleNameKeyword::LowerLatin:
text = String::bijective_base_from(index - 1, String::Case::Lower);
break;
case CSS::CounterStyleNameKeyword::UpperAlpha:
case CSS::CounterStyleNameKeyword::UpperLatin:
text = String::bijective_base_from(index - 1, String::Case::Upper);
break;
case CSS::CounterStyleNameKeyword::LowerRoman:
text = String::roman_number_from(index, String::Case::Lower);
break;
case CSS::CounterStyleNameKeyword::UpperRoman:
text = String::roman_number_from(index, String::Case::Upper);
break;
default:
VERIFY_NOT_REACHED();
}
return MUST(String::formatted("{}. ", text));
},
[](String const& string) -> Optional<String> {
return string;