Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

glossary: update attribute #33386

Merged
merged 3 commits into from
May 7, 2024
Merged

Conversation

PassionPenguin
Copy link
Contributor

@PassionPenguin PassionPenguin commented May 3, 2024

Description

xml attribute behaviour fixes

Motivation

  1. in most compiler/parser, xml attribute with no = and "value" is not allowed and will throw an error of "the spec mandates value for attribute #attr"
  2. in w3c recommendation of XML, attribute is defined as attr eq value

Additional details

  1. w3c https://www.w3.org/TR/xml#sec-starttags
  2. xml validator (searched google and chose first five parser, all throw the same error)

tested that will throw an error:

<tag id />

Related issues and pull requests

@PassionPenguin PassionPenguin requested a review from a team as a code owner May 3, 2024 05:17
@PassionPenguin PassionPenguin requested review from chrisdavidmills and removed request for a team May 3, 2024 05:17
@github-actions github-actions bot added Content:Glossary Glossary entries size/s [PR only] 6-50 LoC changed labels May 3, 2024
Copy link
Contributor

github-actions bot commented May 3, 2024

Preview URLs

Flaws (2)

URL: /en-US/docs/Glossary/Attribute
Title: Attribute
Flaw count: 2

  • macros:
    • /en-US/docs/Web/API/HTMLInputElement/placeholder does not exist
    • /en-US/docs/Web/API/HTMLInputElement/placeholder does not exist
External URLs (1)

URL: /en-US/docs/Glossary/Attribute
Title: Attribute

(comment last updated: 2024-05-07 09:50:09)

Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PassionPenguin nice work! I had a few language suggestions for you, but nothing major.

files/en-us/glossary/attribute/index.md Outdated Show resolved Hide resolved
files/en-us/glossary/attribute/index.md Outdated Show resolved Hide resolved
files/en-us/glossary/attribute/index.md Outdated Show resolved Hide resolved
<input required="" />
<!-- or -->
<input required="required" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surely it is still equivalent to this as well? Include this one as well as the other one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have mixed with the original document (line 13’s removal or the attribute's name in XML.). this is indeed equivalence of html attr=“attr”.

(i’ve checked html spec from + to 5, and in all specs, boolean attribute is true only requires the attribute to exists)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is super interesting — I thought it would have to be required="required" to work, but I tested it and you are correct — it can indeed be required="anything".

Today I learned something ;-)

files/en-us/glossary/attribute/index.md Outdated Show resolved Hide resolved
files/en-us/glossary/attribute/index.md Outdated Show resolved Hide resolved
@PassionPenguin
Copy link
Contributor Author

@PassionPenguin nice work! I had a few language suggestions for you, but nothing major.

thx for your kindly corrections! (sorry for having so many problems in the updation)

PassionPenguin and others added 2 commits May 7, 2024 17:34
Co-authored-by: Chris Mills <chrisdavidmills@gmail.com>
Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great; thank you for your work on this, @PassionPenguin!

@chrisdavidmills chrisdavidmills merged commit 81217ba into mdn:main May 7, 2024
8 checks passed
@PassionPenguin PassionPenguin deleted the xml-attribute branch May 7, 2024 11:03
dalps pushed a commit to dalps/content that referenced this pull request May 8, 2024
* update: xml attribute

* Apply suggestions from code review

Co-authored-by: Chris Mills <chrisdavidmills@gmail.com>

* add equivalence

---------

Co-authored-by: Chris Mills <chrisdavidmills@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Glossary Glossary entries size/s [PR only] 6-50 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants