Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Computed display of form inside table elements should be display:none
https://bugs.webkit.org/show_bug.cgi?id=252541
rdar://problem/105932450

Reviewed by Tim Nguyen.

This patch is to align WebKit with Blink / Chromium and Gecko / Firefox and web-specification.

Web-Spec: https://html.spec.whatwg.org/multipage/rendering.html#tables-2

The patch adds UA stylesheet rule as expected from the web-specification to change display
type of form inside table elements to "display: none" by introducing internal pseudo class ':-internal-html-document'.

* Source/WebCore/css/html.css: Add new UA rules for form in tables
* Source/WebCore/css/CSSSelector.h: New Pseudo class 'PseudoClassHtmlDocument'
* Source/WebCore/css/CSSSelector.cpp:
(CSSSelector::selectorText): As above to translate in ':-internal-html-document'
* Source/WebCore/css/SelectorChecker.cpp:
(SelectorChecker::checkOne): Add case for 'PseudoClassHtmlDocument'
* Source/WebCore/css/SelectorCheckerTestFunctions.h: New bool function 'matchesHtmlDocumentPseudoClass' to return 'HTML' document
* Source/WebCore/css/SelectorPseudoClassAndCompatibilityElementMap.in: Add '-internal-html-document' in list
* Source/WebCore/cssjit/SelectorCompiler.cpp: Add in namespace similar to other
(operationMatchesIsHtmlPseudoClass): New function to return 'matchesHtmlDocumentPseudoClass'
(addPseudoClassType): Add new case 'PseudoClassHtmlDocument'
* Source/WebCore/html/HTMLFormElement.cpp:
(HTMLFormElement::rendererIsNeeded): Deleted hardcoded logic of forms in table layout
(copyNonAttributePropertiesFromElement): Deleted
* Source/WebCore/html/HTMLFormElement.h: Deleted unused definition after above changes
* Source/WebCore/css/parser/CSSSelectorParser.cpp:
(CSSSelectorParser::consumePseudo): Restrict exposing internal pseudo class to web
* Source/WebCore/html/parser/HTMLConstructionSite.cpp:
(HTMLConstructionSite::insertHTMLFormElement): Remove 'isDemoted' and usage
* Source/WebCore/html/parser/HTMLConstructionSite.h: Remove 'isDemoted' from definition
* Source/WebCore/html/parser/HTMLTreeBuilder.cpp:
(HTMLTreeBuilder::processStartTagForInTable): Update since 'isDemoted' is now gone
* LayoutTests/fast/css/pseudo-class-internal.html: Update for new internal pseudo class
* LayoutTests/fast/css/pseudo-class-internal-expected.txt" Update Expectations for above
* LayoutTests/imported/w3c/web-platform-tests/html/rendering/non-replaced-elements/tables/form-in-tables-expected.txt: Rebaselined

Canonical link: https://commits.webkit.org/265283@main
  • Loading branch information
Ahmad-S792 authored and Ahmad Saleem committed Jun 18, 2023
1 parent 1756a62 commit 78b5405
Show file tree
Hide file tree
Showing 16 changed files with 47 additions and 56 deletions.
1 change: 1 addition & 0 deletions LayoutTests/fast/css/pseudo-class-internal-expected.txt
Expand Up @@ -4,6 +4,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE


PASS target.matches(":-internal-direct-focus") threw exception SyntaxError: ':-internal-direct-focus' is not a valid selector..
PASS target.matches(":-internal-html-document") threw exception SyntaxError: ':-internal-html-document' is not a valid selector..
PASS successfullyParsed is true

TEST COMPLETE
Expand Down
1 change: 1 addition & 0 deletions LayoutTests/fast/css/pseudo-class-internal.html
Expand Up @@ -10,6 +10,7 @@
function runTest() {
const internalPseudoClasses = [
':-internal-direct-focus',
':-internal-html-document',
];
for (const pseudo of internalPseudoClasses) {
shouldThrowErrorName('target.matches("' + pseudo + '")', 'SyntaxError');
Expand Down
@@ -1,12 +1,12 @@

FAIL Computed display of form inside TABLE should be 'none' assert_equals: expected "none" but got "block"
FAIL Computed display of form inside THEAD should be 'none' assert_equals: expected "none" but got "block"
FAIL Computed display of form inside TBODY should be 'none' assert_equals: expected "none" but got "block"
FAIL Computed display of form inside TFOOT should be 'none' assert_equals: expected "none" but got "block"
FAIL Computed display of form inside TR should be 'none' assert_equals: expected "none" but got "block"
FAIL Computed display of form inside TABLE should be 'none' (!important UA style)) assert_equals: expected "none" but got "block"
FAIL Computed display of form inside THEAD should be 'none' (!important UA style)) assert_equals: expected "none" but got "block"
FAIL Computed display of form inside TBODY should be 'none' (!important UA style)) assert_equals: expected "none" but got "block"
FAIL Computed display of form inside TFOOT should be 'none' (!important UA style)) assert_equals: expected "none" but got "block"
FAIL Computed display of form inside TR should be 'none' (!important UA style)) assert_equals: expected "none" but got "block"
PASS Computed display of form inside TABLE should be 'none'
PASS Computed display of form inside THEAD should be 'none'
PASS Computed display of form inside TBODY should be 'none'
PASS Computed display of form inside TFOOT should be 'none'
PASS Computed display of form inside TR should be 'none'
PASS Computed display of form inside TABLE should be 'none' (!important UA style))
PASS Computed display of form inside THEAD should be 'none' (!important UA style))
PASS Computed display of form inside TBODY should be 'none' (!important UA style))
PASS Computed display of form inside TFOOT should be 'none' (!important UA style))
PASS Computed display of form inside TR should be 'none' (!important UA style))

3 changes: 3 additions & 0 deletions Source/WebCore/css/CSSSelector.cpp
Expand Up @@ -564,6 +564,9 @@ String CSSSelector::selectorText(StringView separator, StringView rightSide) con
case CSSSelector::PseudoClassInvalid:
builder.append(":invalid");
break;
case CSSSelector::PseudoClassHtmlDocument:
builder.append(":-internal-html-document");
break;
case CSSSelector::PseudoClassLang:
builder.append(":lang(");
ASSERT_WITH_MESSAGE(cs->argumentList() && !cs->argumentList()->isEmpty(), "An empty :lang() is invalid and should never be generated by the parser.");
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/css/CSSSelector.h
Expand Up @@ -128,6 +128,7 @@ struct PossiblyQuotedIdentifier {
PseudoClassFullPageMedia,
PseudoClassDefault,
PseudoClassDisabled,
PseudoClassHtmlDocument, // for internal use only with forms in table case in UA stylesheet
PseudoClassIs,
PseudoClassMatches, // obsolete synonym for PseudoClassIs
PseudoClassWhere,
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/css/SelectorChecker.cpp
Expand Up @@ -1117,6 +1117,9 @@ bool SelectorChecker::checkOne(CheckingContext& checkingContext, const LocalCont
case CSSSelector::PseudoClassHasAttachment:
return hasAttachment(element);
#endif
case CSSSelector::PseudoClassHtmlDocument:
return matchesHtmlDocumentPseudoClass(element);

case CSSSelector::PseudoClassPopoverOpen:
return matchesPopoverOpenPseudoClass(element);

Expand Down
5 changes: 5 additions & 0 deletions Source/WebCore/css/SelectorCheckerTestFunctions.h
Expand Up @@ -562,6 +562,11 @@ ALWAYS_INLINE bool matchesFocusWithinPseudoClass(const Element& element)
return element.hasFocusWithin() && isFrameFocused(element);
}

ALWAYS_INLINE bool matchesHtmlDocumentPseudoClass(const Element& element)
{
return element.document().isHTMLDocument();
}

ALWAYS_INLINE bool matchesModalPseudoClass(const Element& element)
{
if (is<HTMLDialogElement>(element))
Expand Down
@@ -1,4 +1,5 @@
-khtml-drag
-internal-html-document
-webkit-any
-webkit-any-link, PseudoClassAnyLinkDeprecated, PseudoElementUnknown
-webkit-autofill
Expand Down
5 changes: 5 additions & 0 deletions Source/WebCore/css/html.css
Expand Up @@ -299,6 +299,11 @@ caption {
text-align: -webkit-center;
}

/* for table elements with form inside following rules should apply */
:is(table, thead, tbody, tfoot, tr) > form:-internal-html-document {
display: none !important;
}

/* lists */

ul, menu, dir {
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/css/parser/CSSSelectorParser.cpp
Expand Up @@ -768,6 +768,8 @@ std::unique_ptr<CSSParserSelector> CSSSelectorParser::consumePseudo(CSSParserTok
return nullptr;
if (!m_context.hasPseudoClassEnabled && selector->pseudoClassType() == CSSSelector::PseudoClassHas)
return nullptr;
if (m_context.mode != UASheetMode && selector->pseudoClassType() == CSSSelector::PseudoClassHtmlDocument)
return nullptr;
#if ENABLE(ATTACHMENT_ELEMENT)
if (!DeprecatedGlobalSettings::attachmentElementEnabled() && selector->pseudoClassType() == CSSSelector::PseudoClassHasAttachment)
return nullptr;
Expand Down
12 changes: 12 additions & 0 deletions Source/WebCore/cssjit/SelectorCompiler.cpp
Expand Up @@ -125,6 +125,7 @@ namespace SelectorCompiler {
v(operationMatchesLangPseudoClass) \
v(operationMatchesPopoverOpenPseudoClass) \
v(operationMatchesModalPseudoClass) \
v(operationMatchesHtmlDocumentPseudoClass) \
v(operationIsUserInvalid) \
v(operationIsUserValid) \
v(operationAddStyleRelationFunction) \
Expand Down Expand Up @@ -278,6 +279,7 @@ static JSC_DECLARE_JIT_OPERATION_WITHOUT_WTF_INTERNAL(operationMatchesVolumeLock
#if ENABLE(ATTACHMENT_ELEMENT)
static JSC_DECLARE_JIT_OPERATION_WITHOUT_WTF_INTERNAL(operationHasAttachment, bool, (const Element&));
#endif
static JSC_DECLARE_JIT_OPERATION_WITHOUT_WTF_INTERNAL(operationMatchesHtmlDocumentPseudoClass, bool, (const Element&));
static JSC_DECLARE_JIT_OPERATION_WITHOUT_WTF_INTERNAL(operationMatchesPopoverOpenPseudoClass, bool, (const Element&));
static JSC_DECLARE_JIT_OPERATION_WITHOUT_WTF_INTERNAL(operationMatchesModalPseudoClass, bool, (const Element&));
static JSC_DECLARE_JIT_OPERATION_WITHOUT_WTF_INTERNAL(operationIsUserInvalid, bool, (const Element&));
Expand Down Expand Up @@ -1010,6 +1012,12 @@ JSC_DEFINE_JIT_OPERATION(operationMatchesDir, bool, (const Element& element, uin
return element.effectiveTextDirection() == static_cast<TextDirection>(direction);
}

JSC_DEFINE_JIT_OPERATION(operationMatchesHtmlDocumentPseudoClass, bool, (const Element& element))
{
COUNT_SELECTOR_OPERATION(operationMatchesHtmlDocumentPseudoClass);
return matchesHtmlDocumentPseudoClass(element);
}

JSC_DEFINE_JIT_OPERATION(operationMatchesLangPseudoClass, bool, (const Element& element, const FixedVector<PossiblyQuotedIdentifier>& argumentList))
{
COUNT_SELECTOR_OPERATION(operationMatchesLangPseudoClass);
Expand Down Expand Up @@ -1179,6 +1187,10 @@ static inline FunctionType addPseudoClassType(const CSSSelector& selector, Selec
return FunctionType::SimpleSelectorChecker;
#endif

case CSSSelector::PseudoClassHtmlDocument:
fragment.unoptimizedPseudoClasses.append(CodePtr<JSC::OperationPtrTag>(operationMatchesHtmlDocumentPseudoClass));
return FunctionType::SimpleSelectorChecker;

case CSSSelector::PseudoClassPopoverOpen:
fragment.unoptimizedPseudoClasses.append(CodePtr<JSC::OperationPtrTag>(operationMatchesPopoverOpenPseudoClass));
return FunctionType::SimpleSelectorChecker;
Expand Down
36 changes: 0 additions & 36 deletions Source/WebCore/html/HTMLFormElement.cpp
Expand Up @@ -131,36 +131,6 @@ bool HTMLFormElement::formWouldHaveSecureSubmission(const String& url)
return document().completeURL(url).protocolIs("https"_s);
}

bool HTMLFormElement::rendererIsNeeded(const RenderStyle& style)
{
if (!m_wasDemoted)
return HTMLElement::rendererIsNeeded(style);

auto parent = parentNode();
auto parentRenderer = parent->renderer();

if (!parentRenderer)
return false;

// FIXME: Shouldn't we also check for table caption (see |formIsTablePart| below).
bool parentIsTableElementPart = (parentRenderer->isTable() && is<HTMLTableElement>(*parent))
|| (parentRenderer->isTableRow() && parent->hasTagName(trTag))
|| (parentRenderer->isTableSection() && parent->hasTagName(tbodyTag))
|| (parentRenderer->isRenderTableCol() && parent->hasTagName(colTag))
|| (parentRenderer->isTableCell() && parent->hasTagName(trTag));

if (!parentIsTableElementPart)
return true;

DisplayType display = style.display();
bool formIsTablePart = display == DisplayType::Table || display == DisplayType::InlineTable || display == DisplayType::TableRowGroup
|| display == DisplayType::TableHeaderGroup || display == DisplayType::TableFooterGroup || display == DisplayType::TableRow
|| display == DisplayType::TableColumnGroup || display == DisplayType::TableColumn || display == DisplayType::TableCell
|| display == DisplayType::TableCaption;

return formIsTablePart;
}

Node::InsertedIntoAncestorResult HTMLFormElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
{
HTMLElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
Expand Down Expand Up @@ -992,12 +962,6 @@ Vector<Ref<ValidatedFormListedElement>> HTMLFormElement::copyValidatedListedElem
});
}

void HTMLFormElement::copyNonAttributePropertiesFromElement(const Element& source)
{
m_wasDemoted = static_cast<const HTMLFormElement&>(source).m_wasDemoted;
HTMLElement::copyNonAttributePropertiesFromElement(source);
}

HTMLFormElement* HTMLFormElement::findClosestFormAncestor(const Element& startElement)
{
return const_cast<HTMLFormElement*>(ancestorsOfType<HTMLFormElement>(startElement).first());
Expand Down
6 changes: 0 additions & 6 deletions Source/WebCore/html/HTMLFormElement.h
Expand Up @@ -82,8 +82,6 @@ class HTMLFormElement final : public HTMLElement {
ExceptionOr<void> requestSubmit(HTMLElement* submitter);
WEBCORE_EXPORT void reset();

void setDemoted(bool demoted) { m_wasDemoted = demoted; }

void submitImplicitly(Event&, bool fromImplicitSubmissionTrigger);
bool formWouldHaveSecureSubmission(const String& url);

Expand Down Expand Up @@ -131,7 +129,6 @@ class HTMLFormElement final : public HTMLElement {
private:
HTMLFormElement(const QualifiedName&, Document&);

bool rendererIsNeeded(const RenderStyle&) final;
InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) final;
void removedFromAncestor(RemovalType, ContainerNode&) final;
void finishParsingChildren() final;
Expand All @@ -143,8 +140,6 @@ class HTMLFormElement final : public HTMLElement {

void didMoveToNewDocument(Document& oldDocument, Document& newDocument) final;

void copyNonAttributePropertiesFromElement(const Element&) final;

void submit(Event*, bool processingUserGesture, FormSubmissionTrigger, HTMLFormControlElement* submitter = nullptr);

void submitDialog(Ref<FormSubmission>&&);
Expand Down Expand Up @@ -194,7 +189,6 @@ class HTMLFormElement final : public HTMLElement {

bool m_isInResetFunction { false };

bool m_wasDemoted { false };
bool m_isConstructingEntryList { false };
};

Expand Down
3 changes: 1 addition & 2 deletions Source/WebCore/html/parser/HTMLConstructionSite.cpp
Expand Up @@ -509,15 +509,14 @@ void HTMLConstructionSite::insertHTMLBodyElement(AtomHTMLToken&& token)
m_openElements.pushHTMLBodyElement(HTMLStackItem(WTFMove(body), WTFMove(token)));
}

void HTMLConstructionSite::insertHTMLFormElement(AtomHTMLToken&& token, bool isDemoted)
void HTMLConstructionSite::insertHTMLFormElement(AtomHTMLToken&& token)
{
auto element = createHTMLElement(token);
auto& formElement = downcast<HTMLFormElement>(element.get());
// If there is no template element on the stack of open elements, set the
// form element pointer to point to the element created.
if (!openElements().hasTemplateInHTMLScope())
m_form = &formElement;
formElement.setDemoted(isDemoted);
attachLater(currentNode(), formElement);
m_openElements.push(HTMLStackItem(formElement, WTFMove(token)));
}
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/html/parser/HTMLConstructionSite.h
Expand Up @@ -126,7 +126,7 @@ class HTMLConstructionSite {
void insertFormattingElement(AtomHTMLToken&&);
void insertHTMLHeadElement(AtomHTMLToken&&);
void insertHTMLBodyElement(AtomHTMLToken&&);
void insertHTMLFormElement(AtomHTMLToken&&, bool isDemoted = false);
void insertHTMLFormElement(AtomHTMLToken&&);
void insertScriptElement(AtomHTMLToken&&);
void insertTextNode(const String&);
void insertForeignElement(AtomHTMLToken&&, const AtomString& namespaceURI);
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/html/parser/HTMLTreeBuilder.cpp
Expand Up @@ -1085,7 +1085,7 @@ void HTMLTreeBuilder::processStartTagForInTable(AtomHTMLToken&& token)
parseError(token);
if (m_tree.form() && !isParsingTemplateContents())
return;
m_tree.insertHTMLFormElement(WTFMove(token), true);
m_tree.insertHTMLFormElement(WTFMove(token));
m_tree.openElements().pop();
return;
case TagName::template_:
Expand Down

0 comments on commit 78b5405

Please sign in to comment.