Open Bug 2038670 Opened 10 days ago Updated 9 days ago

Should text bounds for an empty input element exclude the frame's padding?

Categories

(Core :: Disability Access APIs, defect)

defect

Tracking

()

People

(Reporter: sajidanwar, Unassigned)

References

(Blocks 2 open bugs)

Details

In patch D292135 for Bug 2029869, I'm changing the implementation of vertical font metrics to have more reasonable synthesized values that align it better with the horizontal font metrics. This has the effect of fixing the size of text carets (previously, the text caret in vertical writing mode was actually not as wide as the text itself). I needed to make a couple changes to the browser_caret_rect.js test to deal with, which was primarily to introduce logical coordinate handling — see my comment on that patch for more details there.

But one issue I found that was causing test instability was the implementation of text range bounds for empty input elements. Bug 1786963 added logic to return the input element's frame bounds when being called on empty inputs, which it says was required for magnifier tools. But the returned frame bounds include the padding of the element. This was causing an assertion in the vertical caret test to fail which expected the text range and the caret to have the same start positions and widths, but which was failing due to the padding-inflated position/widths of the input element. To fix this, I've added padding: 0 to the empty input elements in that test.

The primary question of this bug is: should the empty input element's text range bounds include or exclude the padding? For non-empty text, padding is not included since it returns the bounds of the text itself and not the overall input, so this might be a point in favor of excluding the padding. But on the other side, maybe it's nicer for a magnifier to zoom onto the full element as opposed to just the underlying input area.

I don't think it matters that much, it just needs to be a non-empty rect that roughly matches were the text would go.

Severity: -- → S3
You need to log in before you can comment on or make changes to this bug.