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

Handling of invalid values in menclose notation #144

Closed
fred-wang opened this issue Sep 22, 2019 · 4 comments
Closed

Handling of invalid values in menclose notation #144

fred-wang opened this issue Sep 22, 2019 · 4 comments
Labels
MathML Core Issues affecting the MathML Core specification MathML 4 Issues affecting the MathML 4 specification

Comments

@fred-wang
Copy link

See https://mathml-refresh.github.io/mathml-core/#enclose-expression-inside-notation-menclose

After #104 notation is defined as an unordered set of unique space-separated tokens ; additionally #3 "radical" was removed. The allowed values as "left, right, top, bottom, box, roundedbox, actuarial, madruwb, horizontalstrike, verticalstrike updiagonalstrike, downdiagonalstrike, longdiv, circle.". It think the core spec lacks the usual "If the attribute is absent or has an invalid value, the default 'longiv' value is used"

In MathML3, the notation values are open-ended so an unknown or duplicate values are just ignored. In particular notation="" or notation="invalid" is equivalent to draw nothing and notation="circle circle" to draw an ellipse.

With the current MathML Core text (amended as above), an unknown value or duplicate value will make the attribute invalid and so the 'longdiv' will be used. Hence notation="", notation="invalid", notation="radical" or notation="circle circle" are all equivalent to "longdiv".

I think these are all edge cases, but I just want to be sure that this is what we want.

@fred-wang fred-wang added MathML Core Issues affecting the MathML Core specification MathML 4 Issues affecting the MathML 4 specification need tests Issues related to writing WPT tests need resolution Issues needing resolution at MathML Refresh CG meeting need specification update Issues requiring specification changes labels Sep 22, 2019
@davidcarlisle
Copy link
Collaborator

making it a fixed list (so unexpected entries invalid) is fine with me. The one odd part of your description is that <menclose> with no valid notation attribute acts as longdiv .

box would I think be a more natural default.

I know that's what we said the default was in mathml3 but looking at it now it seems like a strange choice.

I'd be tempted to change the default (in full as well) but if others think that compatibility concerns mean we should stay with longdiv, I wouldn't argue too much.

@fred-wang
Copy link
Author

I don't really have preference about the default, but I would say let's keep longdiv for backward compatibility.

My main concern is that things like notation="", notation="invalid", notation="radical" or notation="circle circle" are now treated as the default while I think in MathML3 they were treated as "no notations drawn" (for the first three) or "circle" (for the fourth one). If people want to preserve the MathML3 behavior, I think the core spec would need to be tweaked a bit.

@fred-wang
Copy link
Author

I updated the spec as agreed during yesterday's meeting, basically keeping backward compat with MathML3

moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 3, 2019
See https://groups.google.com/forum/#!topic/mozilla.dev.platform/vwAkuZIEhnY

* Introduce a new preference option to disable menclose's "radical" notation.
* Disable the notation in Nightly and when running WPT tests.
* Enable the notation in other channels together with a counter and
  deprecation warning.
* Update WPT test legacy-menclose-radical-notation.html
  - Fix test: "radical" should be equivalent to "", which is not the same as
    the default value "longdiv".
    See w3c/mathml#144
  - Add a test "box radical" which should be equivalent to "box".
  - Remove failure expectation.
* Enable the radical notation for MathML reftests testing it.

Differential Revision: https://phabricator.services.mozilla.com/D46721

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1548522
gecko-commit: d2613eaa13690e3771e84ef6ff5080a8fa348a66
gecko-integration-branch: autoland
gecko-reviewers: emilio
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 3, 2019
See https://groups.google.com/forum/#!topic/mozilla.dev.platform/vwAkuZIEhnY

* Introduce a new preference option to disable menclose's "radical" notation.
* Disable the notation in Nightly and when running WPT tests.
* Enable the notation in other channels together with a counter and
  deprecation warning.
* Update WPT test legacy-menclose-radical-notation.html
  - Fix test: "radical" should be equivalent to "", which is not the same as
    the default value "longdiv".
    See w3c/mathml#144
  - Add a test "box radical" which should be equivalent to "box".
  - Remove failure expectation.
* Enable the radical notation for MathML reftests testing it.

Differential Revision: https://phabricator.services.mozilla.com/D46721

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1548522
gecko-commit: 6d734cb4e0cac938e1e3758b44c750f401e0f5b6
gecko-integration-branch: autoland
gecko-reviewers: emilio
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 4, 2019
See https://groups.google.com/forum/#!topic/mozilla.dev.platform/vwAkuZIEhnY

* Introduce a new preference option to disable menclose's "radical" notation.
* Disable the notation in Nightly and when running WPT tests.
* Enable the notation in other channels together with a counter and
  deprecation warning.
* Update WPT test legacy-menclose-radical-notation.html
  - Fix test: "radical" should be equivalent to "", which is not the same as
    the default value "longdiv".
    See w3c/mathml#144
  - Add a test "box radical" which should be equivalent to "box".
  - Remove failure expectation.
* Enable the radical notation for MathML reftests testing it.

Differential Revision: https://phabricator.services.mozilla.com/D46721

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1548522
gecko-commit: 6d734cb4e0cac938e1e3758b44c750f401e0f5b6
gecko-integration-branch: autoland
gecko-reviewers: emilio
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Oct 4, 2019
…=emilio

See https://groups.google.com/forum/#!topic/mozilla.dev.platform/vwAkuZIEhnY

* Introduce a new preference option to disable menclose's "radical" notation.
* Disable the notation in Nightly and when running WPT tests.
* Enable the notation in other channels together with a counter and
  deprecation warning.
* Update WPT test legacy-menclose-radical-notation.html
  - Fix test: "radical" should be equivalent to "", which is not the same as
    the default value "longdiv".
    See w3c/mathml#144
  - Add a test "box radical" which should be equivalent to "box".
  - Remove failure expectation.
* Enable the radical notation for MathML reftests testing it.

Differential Revision: https://phabricator.services.mozilla.com/D46721

--HG--
extra : moz-landing-system : lando
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Oct 4, 2019
…=emilio

See https://groups.google.com/forum/#!topic/mozilla.dev.platform/vwAkuZIEhnY

* Introduce a new preference option to disable menclose's "radical" notation.
* Disable the notation in Nightly and when running WPT tests.
* Enable the notation in other channels together with a counter and
  deprecation warning.
* Update WPT test legacy-menclose-radical-notation.html
  - Fix test: "radical" should be equivalent to "", which is not the same as
    the default value "longdiv".
    See w3c/mathml#144
  - Add a test "box radical" which should be equivalent to "box".
  - Remove failure expectation.
* Enable the radical notation for MathML reftests testing it.

Differential Revision: https://phabricator.services.mozilla.com/D46721

--HG--
extra : moz-landing-system : lando
@fred-wang
Copy link
Author

I added tests for notation="" or notation="radical" since that's needed for Gecko.

I'm closing this issue as I don't think we need it to track more tests... I think this is handled by #105 ; we would need to write a complete test suite for menclose if we decide to keep it.

@fred-wang fred-wang removed need resolution Issues needing resolution at MathML Refresh CG meeting need specification update Issues requiring specification changes need tests Issues related to writing WPT tests labels Oct 4, 2019
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Oct 4, 2019
…=emilio

See https://groups.google.com/forum/#!topic/mozilla.dev.platform/vwAkuZIEhnY

* Introduce a new preference option to disable menclose's "radical" notation.
* Disable the notation in Nightly and when running WPT tests.
* Enable the notation in other channels together with a counter and
  deprecation warning.
* Update WPT test legacy-menclose-radical-notation.html
  - Fix test: "radical" should be equivalent to "", which is not the same as
    the default value "longdiv".
    See w3c/mathml#144
  - Add a test "box radical" which should be equivalent to "box".
  - Remove failure expectation.
* Enable the radical notation for MathML reftests testing it.

Differential Revision: https://phabricator.services.mozilla.com/D46721
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Oct 4, 2019
…=emilio

See https://groups.google.com/forum/#!topic/mozilla.dev.platform/vwAkuZIEhnY

* Introduce a new preference option to disable menclose's "radical" notation.
* Disable the notation in Nightly and when running WPT tests.
* Enable the notation in other channels together with a counter and
  deprecation warning.
* Update WPT test legacy-menclose-radical-notation.html
  - Fix test: "radical" should be equivalent to "", which is not the same as
    the default value "longdiv".
    See w3c/mathml#144
  - Add a test "box radical" which should be equivalent to "box".
  - Remove failure expectation.
* Enable the radical notation for MathML reftests testing it.

Differential Revision: https://phabricator.services.mozilla.com/D46721
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 4, 2019
…=emilio

See https://groups.google.com/forum/#!topic/mozilla.dev.platform/vwAkuZIEhnY

* Introduce a new preference option to disable menclose's "radical" notation.
* Disable the notation in Nightly and when running WPT tests.
* Enable the notation in other channels together with a counter and
  deprecation warning.
* Update WPT test legacy-menclose-radical-notation.html
  - Fix test: "radical" should be equivalent to "", which is not the same as
    the default value "longdiv".
    See w3c/mathml#144
  - Add a test "box radical" which should be equivalent to "box".
  - Remove failure expectation.
* Enable the radical notation for MathML reftests testing it.

Differential Revision: https://phabricator.services.mozilla.com/D46721

UltraBlame original commit: d2613eaa13690e3771e84ef6ff5080a8fa348a66
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 4, 2019
…=emilio

See https://groups.google.com/forum/#!topic/mozilla.dev.platform/vwAkuZIEhnY

* Introduce a new preference option to disable menclose's "radical" notation.
* Disable the notation in Nightly and when running WPT tests.
* Enable the notation in other channels together with a counter and
  deprecation warning.
* Update WPT test legacy-menclose-radical-notation.html
  - Fix test: "radical" should be equivalent to "", which is not the same as
    the default value "longdiv".
    See w3c/mathml#144
  - Add a test "box radical" which should be equivalent to "box".
  - Remove failure expectation.
* Enable the radical notation for MathML reftests testing it.

Differential Revision: https://phabricator.services.mozilla.com/D46721

UltraBlame original commit: 6d734cb4e0cac938e1e3758b44c750f401e0f5b6
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
…=emilio

See https://groups.google.com/forum/#!topic/mozilla.dev.platform/vwAkuZIEhnY

* Introduce a new preference option to disable menclose's "radical" notation.
* Disable the notation in Nightly and when running WPT tests.
* Enable the notation in other channels together with a counter and
  deprecation warning.
* Update WPT test legacy-menclose-radical-notation.html
  - Fix test: "radical" should be equivalent to "", which is not the same as
    the default value "longdiv".
    See w3c/mathml#144
  - Add a test "box radical" which should be equivalent to "box".
  - Remove failure expectation.
* Enable the radical notation for MathML reftests testing it.

Differential Revision: https://phabricator.services.mozilla.com/D46721

UltraBlame original commit: d2613eaa13690e3771e84ef6ff5080a8fa348a66
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
…=emilio

See https://groups.google.com/forum/#!topic/mozilla.dev.platform/vwAkuZIEhnY

* Introduce a new preference option to disable menclose's "radical" notation.
* Disable the notation in Nightly and when running WPT tests.
* Enable the notation in other channels together with a counter and
  deprecation warning.
* Update WPT test legacy-menclose-radical-notation.html
  - Fix test: "radical" should be equivalent to "", which is not the same as
    the default value "longdiv".
    See w3c/mathml#144
  - Add a test "box radical" which should be equivalent to "box".
  - Remove failure expectation.
* Enable the radical notation for MathML reftests testing it.

Differential Revision: https://phabricator.services.mozilla.com/D46721

UltraBlame original commit: 6d734cb4e0cac938e1e3758b44c750f401e0f5b6
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 5, 2019
…=emilio

See https://groups.google.com/forum/#!topic/mozilla.dev.platform/vwAkuZIEhnY

* Introduce a new preference option to disable menclose's "radical" notation.
* Disable the notation in Nightly and when running WPT tests.
* Enable the notation in other channels together with a counter and
  deprecation warning.
* Update WPT test legacy-menclose-radical-notation.html
  - Fix test: "radical" should be equivalent to "", which is not the same as
    the default value "longdiv".
    See w3c/mathml#144
  - Add a test "box radical" which should be equivalent to "box".
  - Remove failure expectation.
* Enable the radical notation for MathML reftests testing it.

Differential Revision: https://phabricator.services.mozilla.com/D46721

UltraBlame original commit: d2613eaa13690e3771e84ef6ff5080a8fa348a66
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 5, 2019
…=emilio

See https://groups.google.com/forum/#!topic/mozilla.dev.platform/vwAkuZIEhnY

* Introduce a new preference option to disable menclose's "radical" notation.
* Disable the notation in Nightly and when running WPT tests.
* Enable the notation in other channels together with a counter and
  deprecation warning.
* Update WPT test legacy-menclose-radical-notation.html
  - Fix test: "radical" should be equivalent to "", which is not the same as
    the default value "longdiv".
    See w3c/mathml#144
  - Add a test "box radical" which should be equivalent to "box".
  - Remove failure expectation.
* Enable the radical notation for MathML reftests testing it.

Differential Revision: https://phabricator.services.mozilla.com/D46721

UltraBlame original commit: 6d734cb4e0cac938e1e3758b44c750f401e0f5b6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MathML Core Issues affecting the MathML Core specification MathML 4 Issues affecting the MathML 4 specification
Projects
None yet
Development

No branches or pull requests

2 participants