Skip to content

Commit

Permalink
Bug 1830909 - Implement <hr> in <select> r=hsivonen,emilio,geckoview-…
Browse files Browse the repository at this point in the history 
…reviewers,desktop-theme-reviewers,Jamie,owlish

Updated HTML parser to allow <hr> in <select>.

Updated internal toolkit UI for <select> dropdown to create
menuseperators for hrs.

Updated WPT expectations:
 - HTML5Lib WebKit parsing for it now passes 100%

Also includes Android support, but Fenix does not support separators
in the menus used (single/multiple) yet so they are not rendered.

Differential Revision: https://phabricator.services.mozilla.com/D189065
  • Loading branch information
@CanadaHonk
CanadaHonk committed Nov 28, 2023
1 parent 289e9fb commit a1eb267
Show file tree
Hide file tree
Showing 10 changed files with 186 additions and 117 deletions.
2 changes: 2 additions & 0 deletions browser/base/content/test/forms/browser.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,5 @@ fail-if = ["a11y_checks"] # Bug 1854233 select may not be labeled
["browser_selectpopup_width.js"]

["browser_selectpopup_xhtml.js"]

["browser_selectpopup_hr.js"]
55 changes: 55 additions & 0 deletions browser/base/content/test/forms/browser_selectpopup_hr.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
add_task(async function test_hr() {
await SpecialPowers.pushPrefEnv({
set: [["dom.forms.select.customstyling", true]],
});

const PAGE_CONTENT = `
<!doctype html>
<select>
<option>One</option>
<hr style="color: red; background-color: blue">
<option>Two</option>
</select>`;

const pageUrl = "data:text/html," + encodeURIComponent(PAGE_CONTENT);
let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, pageUrl);

const selectPopup = await openSelectPopup("click");
const menulist = selectPopup.parentNode;

const optionOne = selectPopup.children[0];
const separator = selectPopup.children[1];
const optionTwo = selectPopup.children[2];

is(optionOne.textContent, "One", "First option has expected text content");

is(separator.tagName, "menuseparator", "Separator is menuseparator");

const separatorStyle = getComputedStyle(separator);

is(
separatorStyle.color,
"rgb(255, 0, 0)",
"Separator color is specified CSS color"
);

is(
separatorStyle.backgroundColor,
"rgba(0, 0, 0, 0)",
"Separator background-color is not set to specified CSS color"
);

is(optionTwo.textContent, "Two", "Second option has expected text content");

is(menulist.activeChild, optionOne, "First option is selected to start");

EventUtils.synthesizeKey("KEY_ArrowDown");

is(
menulist.activeChild,
optionTwo,
"Second option is selected after arrow down"
);

BrowserTestUtils.removeTab(tab);
});
2 changes: 2 additions & 0 deletions mobile/android/components/geckoview/GeckoViewPrompt.sys.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ export class PromptFactory {
} else if (win.HTMLOptionElement.isInstance(child)) {
item.label = child.label || child.text;
item.selected = child.selected;
} else if (win.HTMLHRElement.isInstance(child)) {
item.separator = true;
} else {
continue;
}
Expand Down
33 changes: 33 additions & 0 deletions parser/html/javasrc/TreeBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2573,6 +2573,20 @@ public final void startTag(ElementName elementName,
startTagTemplateInHead(elementName, attributes);
attributes = null; // CPP
break starttagloop;
case HR:
if (isCurrent("option")) {
pop();
}
if (isCurrent("optgroup")) {
pop();
}
appendVoidElementToCurrent(elementName, attributes);
selfClosing = false;
// [NOCPP[
voidElement = true;
// ]NOCPP]
attributes = null; // CPP
break starttagloop;
default:
errStrayStartTag(name);
break starttagloop;
Expand Down Expand Up @@ -5457,6 +5471,25 @@ private void appendToCurrentNodeAndPushElementMayFoster(ElementName elementName,
push(node);
}

private void appendVoidElementToCurrent(
ElementName elementName, HtmlAttributes attributes)
throws SAXException {
@Local String popName = elementName.getName();
// [NOCPP[
checkAttributes(attributes, "http://www.w3.org/1999/xhtml");
if (!elementName.isInterned()) {
popName = checkPopName(popName);
}
// ]NOCPP]
T currentNode = nodeFromStackWithBlinkCompat(currentPtr);
T elt = createElement("http://www.w3.org/1999/xhtml", popName, attributes, currentNode
// CPPONLY: , htmlCreator(elementName.getHtmlCreator())
);
appendElement(elt, currentNode);
elementPushed("http://www.w3.org/1999/xhtml", popName, elt);
elementPopped("http://www.w3.org/1999/xhtml", popName, elt);
}

private void appendVoidElementToCurrentMayFoster(
ElementName elementName, HtmlAttributes attributes, T form) throws SAXException {
@Local String name = elementName.getName();
Expand Down
24 changes: 24 additions & 0 deletions parser/html/nsHtml5TreeBuilder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1759,6 +1759,18 @@ void nsHtml5TreeBuilder::startTag(nsHtml5ElementName* elementName,
attributes = nullptr;
NS_HTML5_BREAK(starttagloop);
}
case HR: {
if (isCurrent(nsGkAtoms::option)) {
pop();
}
if (isCurrent(nsGkAtoms::optgroup)) {
pop();
}
appendVoidElementToCurrent(elementName, attributes);
selfClosing = false;
attributes = nullptr;
NS_HTML5_BREAK(starttagloop);
}
default: {
errStrayStartTag(name);
NS_HTML5_BREAK(starttagloop);
Expand Down Expand Up @@ -4311,6 +4323,18 @@ void nsHtml5TreeBuilder::appendToCurrentNodeAndPushElementMayFoster(
push(node);
}

void nsHtml5TreeBuilder::appendVoidElementToCurrent(
nsHtml5ElementName* elementName, nsHtml5HtmlAttributes* attributes) {
nsAtom* popName = elementName->getName();
nsIContentHandle* currentNode = nodeFromStackWithBlinkCompat(currentPtr);
nsIContentHandle* elt =
createElement(kNameSpaceID_XHTML, popName, attributes, currentNode,
htmlCreator(elementName->getHtmlCreator()));
appendElement(elt, currentNode);
elementPushed(kNameSpaceID_XHTML, popName, elt);
elementPopped(kNameSpaceID_XHTML, popName, elt);
}

void nsHtml5TreeBuilder::appendVoidElementToCurrentMayFoster(
nsHtml5ElementName* elementName, nsHtml5HtmlAttributes* attributes,
nsIContentHandle* form) {
Expand Down
2 changes: 2 additions & 0 deletions parser/html/nsHtml5TreeBuilder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,8 @@ class nsHtml5TreeBuilder : public nsAHtml5TreeBuilderState {
void appendToCurrentNodeAndPushElementMayFoster(
nsHtml5ElementName* elementName, nsHtml5HtmlAttributes* attributes,
nsIContentHandle* form);
void appendVoidElementToCurrent(nsHtml5ElementName* elementName,
nsHtml5HtmlAttributes* attributes);
void appendVoidElementToCurrentMayFoster(nsHtml5ElementName* elementName,
nsHtml5HtmlAttributes* attributes,
nsIContentHandle* form);
Expand Down
100 changes: 0 additions & 100 deletions testing/web-platform/meta/html/syntax/parsing/html5lib_webkit02.html.ini
View file

This file was deleted.

29 changes: 27 additions & 2 deletions toolkit/actors/SelectChild.sys.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,17 +382,43 @@ function uniqueStylesIndex(cs, uniqueStyles) {
function buildOptionListForChildren(node, uniqueStyles) {
let result = [];

let lastWasHR = false;
for (let child of node.children) {
let className = ChromeUtils.getClassName(child);
let isOption = className == "HTMLOptionElement";
let isOptGroup = className == "HTMLOptGroupElement";
if (!isOption && !isOptGroup) {
let isHR = className == "HTMLHRElement";
if (!isOption && !isOptGroup && !isHR) {
continue;
}
if (child.hidden) {
continue;
}

let cs = getComputedStyles(child);

if (isHR) {
// https://html.spec.whatwg.org/#the-select-element-2
// "Each sequence of one or more child hr element siblings may be rendered as a single separator."
if (lastWasHR) {
continue;
}

const defaultHRStyle = node.ownerGlobal.getDefaultComputedStyle(child);
let info = {
index: child.index,
display: cs.display,
isHR,
styleIndex: uniqueStylesIndex(cs, uniqueStyles),
defaultHRStyle: supportedStyles(defaultHRStyle, ["color"]),
};
result.push(info);

lastWasHR = true;
continue;
}
lastWasHR = false;

// The option code-path should match HTMLOptionElement::GetRenderedLabel.
let textContent = isOptGroup
? child.getAttribute("label")
Expand All @@ -401,7 +427,6 @@ function buildOptionListForChildren(node, uniqueStyles) {
textContent = "";
}

let cs = getComputedStyles(child);
let info = {
index: child.index,
isOptGroup,
Expand Down
51 changes: 36 additions & 15 deletions toolkit/actors/SelectParent.sys.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,13 @@ export var SelectParentHelper = {
let inactiveRule = null;
for (const property of SUPPORTED_OPTION_OPTGROUP_PROPERTIES) {
let shouldSkip = (function () {
if (items[i].isHR) {
if (property == "color") {
return style.color == items[i].defaultHRStyle.color;
}
return true;
}

if (property == "direction" || property == "font-size") {
// Handled elsewhere.
return true;
Expand Down Expand Up @@ -489,26 +496,20 @@ export var SelectParentHelper = {
let ariaOwns = "";
for (let option of options) {
let isOptGroup = option.isOptGroup;
let item = element.ownerDocument.createXULElement(
isOptGroup ? "menucaption" : "menuitem"
);
let isHR = option.isHR;

let xulElement = "menuitem";
if (isOptGroup) {
item.setAttribute("role", "group");
xulElement = "menucaption";
}
item.setAttribute("label", option.textContent);
if (isHR) {
xulElement = "menuseparator";
}

let item = element.ownerDocument.createXULElement(xulElement);
item.className = `ContentSelectDropdown-item-${option.styleIndex}`;
item.hidden =
option.display == "none" || (parentElement && parentElement.hidden);
// Keep track of which options are hidden by page content, so we can avoid
// showing them on search input.
item.hiddenByContent = item.hidden;
item.setAttribute("tooltiptext", option.tooltip);

if (uniqueOptionStyles[option.styleIndex].customStyling) {
item.setAttribute("customoptionstyling", "true");
} else {
item.removeAttribute("customoptionstyling");
}

if (parentElement) {
// In the menupopup, the optgroup is a sibling of its contained options.
Expand All @@ -523,6 +524,26 @@ export var SelectParentHelper = {
element.appendChild(item);
nthChildIndex++;

if (isHR) {
// Continue early as HRs do not have other attributes.
continue;
}

if (isOptGroup) {
item.setAttribute("role", "group");
}
item.setAttribute("label", option.textContent);
// Keep track of which options are hidden by page content, so we can avoid
// showing them on search input.
item.hiddenByContent = item.hidden;
item.setAttribute("tooltiptext", option.tooltip);

if (uniqueOptionStyles[option.styleIndex].customStyling) {
item.setAttribute("customoptionstyling", "true");
} else {
item.removeAttribute("customoptionstyling");
}

// A disabled optgroup disables all of its child options.
let isDisabled = isGroupDisabled || option.disabled;
if (isDisabled) {
Expand Down
Loading

0 comments on commit a1eb267

Please sign in to comment.