w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.51k stars 669 forks source link

[css-syntax] Should numbers have type `integer` whenever the actual value is an integer? #10238

Open romainmenke opened 7 months ago

romainmenke commented 7 months ago

See : https://drafts.csswg.org/css-syntax/#consume-number

Also mentioned in passing by @tabatkins here : https://github.com/w3c/csswg-drafts/issues/6471#issuecomment-885945731

As a result of the current tokenizing rules these values have type number :

While both arguably represent integer values. Should the algorithm be updated to reflect this?

You can see the difference between numbers with type number and integer in action here : https://codepen.io/romainmenke/pen/GRLwLQy

Wrapping values of type number in a math expression already "normalizes" these into integers when possible. (e.g. z-index: calc(10e3) works)

romainmenke commented 7 months ago

@cdoublev said :

I think <integer> only exists for some corner cases of the CSS grammar. There should be a default rounding behavior in properties like order, z-index, etc.

Another argument is that Number.isInteger(1.0) is truthy, probably like in most if not all progaming languages.

fantasai commented 6 months ago

Anything with a decimal point should not be parsed as <integer>, imho.

tabatkins commented 6 months ago

Agreed on that point. The only real point of contention, I think, is whether sci-not can produce integers - both 1e3 (very reasonable) and also 1.2e3 (equivalent to 1200, so maybe?)

cdoublev commented 6 months ago

How does producing an integer from scientific notation differ from producing it from decimal notation? It seems that both depend on evaluating the mathematical value of the input number. I have no math/cs degree so please forgive me if the answer is obvious.

Proposed: only produce <integer-token> from signed/unsigned digits, other values are <number-token>, which means only <number-token> would serialize with sci-not and :nth-child(1e1) would remain invalid.

Loirooriol commented 6 months ago

IMO if 5.0 isn't an integer, it would be weird for 5.0e0 to be an integer.

romainmenke commented 6 months ago

https://drafts.csswg.org/cssom/#serializing-css-values

Could say that anything that serializes into an integer is actually an integer. That can't be the spec text, but conceptually that has few surprises.

Loirooriol commented 6 months ago

Note that spec needs edits: https://github.com/w3c/csswg-drafts/issues/8538#issuecomment-1523755872

tabatkins commented 6 months ago

Ah right, we are still missing that edit, partially because of this issue (and partially because I forgot about it).

Okay, proposal:

A number is an integer if it either:

The serialization edit from #8538 will satisfy this; since we serialize to no more than 6 digits after the decimal point, and scinot doesn't kick in for large number until it's substantially larger than a million, any large-number scinot will necessarily be parsed as an integer.

fantasai commented 5 months ago

I might suggest dropping that last rule, I think it simplifies both the parsing rules and the mental model (No decimal points, no scinot that generates decimal points).

LeaVerou commented 5 months ago

+1 to what is proposed here, I have personally run into many issues where this was a problem, and I know it's common enough that people are employing crazy hacks like assigning the <number> to a dummy registered --integer property.

However, I’d go even further: instead of debating whether a <number> should be considered an <integer> if it’s an integer, IMO a better solution would be to allow a <number> to be used anywhere an <integer> is expected, and just be converted to an <integer> using some default rounding strategy (probably floor, which is what assigning to --integer does today). Then the point discussed here becomes moot as there are very few cases where it still makes a difference.

tabatkins commented 5 months ago

I might suggest dropping that last rule, I think it simplifies both the parsing rules and the mental model (No decimal points, no scinot that generates decimal points).

Without that last point, you can't re-use numeric serialization code between JS and CSS (and other contexts). It's not uncommon to get large integers serialized with a decimal component and a large enough exponent to ensure it's a decimal.

IMO a better solution would be to allow a <number> to be used anywhere an <integer> is expected, and just be converted to an <integer> using some default rounding strategy

That would be a compat-risky change, as it would mean the properties and values that currently take integers, but might be written with a non-integer and currently ignored for invalidity, would become valid suddenly.

But also, it's separate from this change - regardless of whether we allow numbers where integers are expected, we can still define more things to be integers.

Loirooriol commented 5 months ago

It seems a bit inconsistent that 1.2e1 is an integer, but 120e-1 is not?

LeaVerou commented 5 months ago

That would be a compat-risky change, as it would mean the properties and values that currently take integers, but might be written with a non-integer and currently ignored for invalidity, would become valid suddenly.

Sure, but making invalid things valid is something we do all the time. We could do some compat analysis and see if it actually breaks anything. I think it would save authors from a lot of confusing mistakes.

css-meeting-bot commented 5 months ago

The CSS Working Group just discussed [css-syntax] Should numbers have type `integer` whenever the actual value is an integer?.

The full IRC log of that discussion <jarhar> TabAtkins: question was raised if syntax spec should - currently distinguish bettween tokens parsed as number and some as integer
<jarhar> TabAtkins: rule at the moment is if its just digits its an integer its ?, exponent its a number
<jarhar> TabAtkins: implications fo rproperties like z index, invalid to do z index of 1 e 3 instead of ?
<jarhar> TabAtkins: romain was asking if we should treat things as integers if they are an integer value
<jarhar> TabAtkins: ? large enough that we are doing an integer part, stuff after decimal was zero
<jarhar> TabAtkins: i suspect we cannot change ? after decimal point, thats been a number for a long time and we sohould keep that, clear that you want a float
<jarhar> TabAtkins: we should relax scifi rules because you cant integer scifi notation ever
<TabAtkins> rhttps://github.com/w3c/csswg-drafts/issues/10238#issuecomment-2080095998
<fantasai> s/rhttp/http/
<oriol> q+
<jarhar> TabAtkins: proposal is that a ? integer if you doesn't have a decimal or just digits, if it doesn't have a decimal or zero or positive scientific notation, or it has a decimal compoentn, or it has number of points after the decimal, so like 1.1e2 would be an integer because its 110, but 1.11e1 would not be because its 11.1
<jarhar> TabAtkins: that would also work for the serialization rules from that are already defined in scientific notation, for large integers numbers with decimal points and a big exponent
<jarhar> TabAtkins: and i think thats going to be safe. only case with giant integers is z index and we want serialization of z index to be usable
<jarhar> TabAtkins: might not round trip in some cases today
<jarhar> TabAtkins: if theres nothing that tries to indicate a less than one segment of the number, otherwise its a number, how does that sound
<fserb> q+
<astearns> ack oriol
<jarhar> oriol: i was wondering if its inconsistent that in one case we are comparing number of decimals vs exponent, on the other hand if the exponent is negative then we dont ? about trailing zeros. if we have 120e-1 then we get 12 but we do not consider this to be an integer according to proposal
<jarhar> TabAtkins: i have no opinion. nobody writes that, i simply dont care. if we want to go either direction on those
<astearns> ack fserb
<jarhar> fserb: you said that the only use case is this a problem is z index
<jarhar> TabAtkins: that is an integer value property that often has big numbers
<florian> q?
<jarhar> fserb: do we have a good reason to not allow 5.0? i konw its not an integer, we're creating a rule about numbers that feels arbitrary. if the case is just because z index 5.0 its a horrible thing then you should just let it slide and forget about the notion of an integer
<jarhar> astearns: its a compat thing, there is code that expects it to see 5.0
<jarhar> TabAtkins: and theres languages that distinguish between numberic types, they treat 5.0 as a non integer
<jarhar> fserb: yes but they also have ways to convert float
<astearns> s/see 5.0/see 5.0 as a number/
<jarhar> TabAtkins: css does too, wrap in calc
<jarhar> fserb: isn't that silly?
<jarhar> TabAtkins: it rounds its fine
<jarhar> fserb: shouldn't we just live with z index 5.0 and be happy?
<jarhar> fantasai: we can't do that for compat reasons, no need to debate whether its a good idea
<jarhar> fserb: on that proposal i just left a comment, yhou said the same thing you wrote. there is also the use case where you have more than one digit 50e-1 should also be an integer
<jarhar> TabAtkins: it doesn't follow. we can put it in, if not whatever
<astearns> q?
<astearns> ack fantasai
<jarhar> fantasai: i think theres two approaches we can go here. one is that you figure out whether the ifinal result of a ? the scientific notation has decimals or not. and then decide whether its an integer ot not or
<jarhar> fantasai: you can go from a more parsing perspetive of does it have a negative exponent or decimal and if its no its an integer. as oriol pointed out youre doing half an dhalf
<jarhar> TabAtkins: doesn't work serialization of large decimals produces a decimal point with a large expont
<jarhar> fantasai: i dont understand why
<jarhar> TabAtkins: once you start serializing with scientific notation, langagues serialize with ? decimal pointa nd then put everything behind the decimal point. js does it, we approved the serizliation rules, other ? you use serialize the same way. it uses one ? digit and a bunch of decimal digits
<astearns> ack dbaron
<jarhar> dbaron: i was on the queue to say why i like tabs proposal and i think that might help answer this concern. my undesrstanding of what tab proposed is that its just a parse time - im going to explain the same thing in a differnet way
<jarhar> dbaron: the way i understand it is that it is purely a parse time thing but youre asking did you write any digits in the number part not the exponent part whose maeaning after you apply thye exponent is that they go in a less than one position
<jarhar> dbaron: its purely syntactic. the qeustion is did you write any digits after the decima. point, except that we mean where the logical decimal point is after the exponent. when you use a negative exponent, it is impossible to write ? after the digits? if your exponent is negative 1, the lst digit is after the decimal point digit
<jarhar> dbaron: my way of thinking what tabs proposing, its an integer if theres no digits at the .1 position or the .01 position, if ? ten position or hundred position or higher
<jarhar> dbaron: did you write any digits at those positions ? only thing that makes this ?
<jarhar> TabAtkins: that is the logic behind the rules yes
<florian> q?
<fserb> q+
<jarhar> dbaron: maybe you could think of it as its aparsing thing on the number before the thing but a logical thing about the number after the e
<jarhar> florian: ? equal to 1.0 and thats not an integer
<astearns> ack fserb
<TabAtkins> s/?/10e-1/
<jarhar> florian: problem by serializing with scientific notation in the first place, tabs proposal makes sense to me given thats the case
<jarhar> fantasai: alternative would be to not serialize with scientific notation?
<jarhar> TabAtkins: we already have widepsread usage of that
<jarhar> fantasai: but it doesn't rountrip right?
<florian> s/problem by serializing/we got into this problem by serializing
<jarhar> TabAtkins: its broken right now but we produce scientific notation. very easy to get scientific notation into your css by accident via cssom methods. it will string them into that if theyre big
<jarhar> TabAtkins: thats why we need sign nodes anywya, but its easy to produce it via js and hard to avoid doing that
<TabAtkins> s/sign nodes/scinot/
<jarhar> astearns: proposed resolution is to add this parse time evaluation of scientific notation as integers
frivoal commented 5 months ago

Sure, but making invalid things valid is something we do all the time. We could do some compat analysis and see if it actually breaks anything. I think it would save authors from a lot of confusing mistakes.

On z-index, I would be surprised if it didn't break things. People through random weird z-indexes at things to try and solve problems all the time, and often it doesn't work, and then they do something else but leave the inappropriate z-index behind. causing previously invalid z-index to become valid seems very likely to break things.

astearns commented 5 months ago

I think we actually resolved on this, but it was not reflected in the minutes. Does anyone else recall?

RESOLVED: add this parse time evaluation of scientific notation as integers