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

nesting description nodes allowed or not? #210

Closed
hsitter opened this issue Dec 5, 2018 · 1 comment
Closed

nesting description nodes allowed or not? #210

hsitter opened this issue Dec 5, 2018 · 1 comment

Comments

@hsitter
Copy link
Contributor

hsitter commented Dec 5, 2018

in a kde appdata we had the following:

  <description>
    <p>
      KCalc has everything you would expect from a scientific calculator, plus:
      <ul>
        <li>Trigonometric functions, logic operations and statistical calculations</li>
        <li>A results stack which enables convenient recall of previous calculation results</li>
        <li>Precision is user-definable</li>
        <li>The display allows cut and paste of numbers</li>
        <li>The display colors and font are configurable, aiding usability</li>
        <li>The use of key-bindings make it easy to use without a pointing device</li>
      </ul>
    </p>
  </description>

That is: <ul> nested inside <p>.

Appstream validated that fine via appstreamcli validate but people have raised concern over the correctness of this in particular WRT localization. The appstream documentation's examples seem to always close the p node before starting a list node. At the same time I cannot find any concrete information that would suggest this type of nesting is not allowed.

So, is it correct that <p><ul/></p> validates as correct?

If not I am guessing the validation should get refined a bit.

(background: https://markmail.org/thread/ewarmwjcojp43qqt)

@ximion
Copy link
Owner

ximion commented Dec 5, 2018

So, is it correct that <p><ul/></p> validates as correct?

Short answer: No. Nested listings in <p/> tags are not allowed, and the validator really should catch that.
I wonder how AppStream would actually parse such a tag... I guess currently the nested <ul/> would just silently be ignored.
I'll look into refining the validator check, unless someone is faster than me with a PR (might take me a few days to get to it).
Thank you for raising this issue!

@ximion ximion closed this as completed in bc2ddec Dec 22, 2018
klausenbusk added a commit to flathub/com.airtame.Client that referenced this issue Feb 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants