The Safari bug that never was (2022)

> I think I see. There's a block at the bottom of RenderText::computePreferredLogicalWidths() which is only hit if wordLen == 0 and !isNewline (which is the case here) which measures the width of the one character at index i and adds it to currMaxWidth. In our situation, that's the leading newline character.

> Then, later, we subtract it out like this: "widths.max -= font.width(RenderBlock::constructTextRun(&space, 1, style));". This expects that the width of this initial character is equal to the width of a space.

“In HTML, any extra whitespace between words is ignored. This means you can add extra spaces, and even new lines, between words without affecting how they are displayed in the browser”

So, “ignoring the new line and just assuming it was the same width as a space” is correct behaviour.

I now think what happened is this:

- the width of the box to render the text in was computed correctly

- the code that then laid out the text inside that box incorrectly used the width of the newline in the HTML to compute the length of the text, concluded the text was too long to fit on a line, and line-wrapped it inside the box.

That’s not consistent with the claim from the article “and so the box that it made was too small”, though.

Alternatively, the font’s line feed glyph was _less_wide_ than a space, and the width of the box was computed as too small, but then the article is incorrect in that it claims the font’s line feed glyph was _wider_ than a space.

(and for those wondering why a browser needs to compute the width of that text twice: I wouldn’t know, but browser layout engines are complex beasts)

> Reviewed by NOBODY (OOPS!).

If I was reviewing the code, I would have asked why Myles changed WidthIterator.cpp's call to charactersTreatedAsSpace.constructAndAppend from a separate parameter on each line to everything inline.

This change made the diff of this change harder to read, and will make the diff of any future parameter change harder to read also.

I also would have created an intermediary variable or a helper method for `character == tabCharacter ? width : font.spaceWidth()` - it's not clear at all here why this check needs to be there (without looking at the ChangeLog, which I shouldn't have to do to understand code), or why the old behaviour that used `width` instead of `font.spaceWidth()` ONLY applies to tabCharacter (So it should be encapsulate inside a method with a clear comment with the info from the ChangeLog)

Lastly, this diff doesn't seem to match the blog description:

> The font also has data about the newline character, including its width. This doesn’t really make sense – new lines don’t (or at least shouldn’t) take up any space, but the font doesn’t treat it differently to any other character. The creator of the font still has to include a width for the new line character in the font’s data. And in the font we use on GOV.UK, the new line character has a bigger width than a space character – which is apparently unusual.

This makes it sound like the FONT is wrong and the font width shouldn't be used for width calculation. Yet the change in the code introduces using the font's space width explicitly.

Ultimately the ChangeLog doesn't match the code. I trust the blog writer, the code author, and the description of the bug in the ChangeLog. What needed improvement here then is the code to imperatively match what is described in English.

No, this is not true. Radar is not just "a bug tracker" — it is a very powerful project management tool, custom-built over several decades for the way Apple works, with integrations into other internal tools and processes.

It should be obvious why Apple wants to track work being done on Webkit using the same tool it uses to track work being done on every other project at the company.