Fix #12193 Servo displays upper level Thai character in wrong place.

[veer66]

---

• ./mach build -d does not report any errors
• ./mach test-tidy does not report any errors
• These changes fix #__ (github issue number if applicable).

• There are tests for these changes OR
• These changes do not require tests because _____

---

This change is

[highfive]

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @pcwalton (or someone else) soon.

[highfive]

Warning

• These commits modify gfx code, but no tests are modified. Please consider adding a test!

[pcwalton]

Great find! Could you add a reftest for this to make sure it doesn't regress? Thanks :)

[emilio]

@bors-servo: try

[bors-servo]

⌛ Trying commit 2f5830e with merge 5046ca6...

[bors-servo]

💔 Test failed - linux-rel

[highfive]

  ▶ FAIL [expected PASS] /css-text-3_dev/html/word-break-break-all-007.htm
  └   → /css-text-3_dev/html/word-break-break-all-007.htm 0a209e6461d4f5e3047d63c8352b630365afa2b3
/css-text-3_dev/html/reference/word-break-break-all-ref-007.htm e646b685dec341a3d6988b6e3f52423167e70952
Testing 0a209e6461d4f5e3047d63c8352b630365afa2b3 == e646b685dec341a3d6988b6e3f52423167e70952

  ▶ FAIL [expected PASS] /css-transforms-1_dev/html/perspective-origin-003.htm
  └   → /css-transforms-1_dev/html/perspective-origin-003.htm e79c0e9b3e8e596c5f445a33630e8d2333d30ec4
/css-transforms-1_dev/html/reference/ref-filled-green-100px-square.htm d4aa213e3e31e41d0d8e84aec79e94f860f27178
Testing e79c0e9b3e8e596c5f445a33630e8d2333d30ec4 == d4aa213e3e31e41d0d8e84aec79e94f860f27178

[emilio]

No luck, there wasn't a pre-existing test covering this, so you should create one as @pcwalton says.

The word-break-break-all-007.htm test doesn't seem intermittent, and seems related to this PR, so it deserves some investigation too.

You don't need to do anything about the perspective test, that's #12891.

Thanks for your PR! :)

[veer66]

A reftest was just added.

[emilio]

@bors-servo: r=pcwalton,emilio

[bors-servo]

📌 Commit 99551ea has been approved by pcwalton,emilio

[bors-servo]

⌛ Testing commit 99551ea with merge 954ab56...

[KiChjang]

What about the test failure for word-break-break-all-007.htm? Is that not addressed?

[emilio]

@KiChjang Hmm... I thought it was intermittent, but apparently it's not? Then it'll fail.

@bors-servo: r-

Yeah, it deserves some more investigation, @mbrubeck seemed to think they were passing accidentally in #11303, but yeah, agreed. @veer66 can you check if they were passing by accident before your patch or not?

[bors-servo]

💔 Test failed - mac-rel-css

[highfive]

  ▶ FAIL [expected PASS] /css-text-3_dev/html/word-break-break-all-007.htm
  └   → /css-text-3_dev/html/word-break-break-all-007.htm eb8cff0836df20e6e83717fc15edb9123a68a3d7
/css-text-3_dev/html/reference/word-break-break-all-ref-007.htm fc6915d2794cb9c74615636313f9c5d1a21fbee1
Testing eb8cff0836df20e6e83717fc15edb9123a68a3d7 == fc6915d2794cb9c74615636313f9c5d1a21fbee1

[veer66]

According to /word-break-break-all-007.htm, after applying the patch, document.getElementById('testspan').offsetWidth returns 0.

Please give a hint where the code for obtaining offsetWidth is.

[emilio]

This is what you're looking for:

servo/components/layout/query.rs

Line 777 in 4cefbcc

pub fn process_offset_parent_query<N: LayoutNode>(requested_node: N, layout_root: &mut FlowRef)

Thanks!

[veer66]

document.getElementById('testspan').offsetWidth returned 0 for /word-break-break-all-007.htm even before the patch was applied. I keep investigating.

[veer66]

word-break-break-all-007.htm test should fail before apply patch.

By increasing div.test width to 420px,

In Servo (2eb0328), it looks like picture below:

However, I should look like this:

[veer66]

When I just change width of div in word-break-break-all-007.htm to 370px, the test can be passed.

In brief, the problem occurs because document.getElementById('testspan').offsetWidth always return 0.

[emilio]

It seems to me we might have been passing that test accidentally, but I'll let @pcwalton confirm if that's the case.

[pcwalton]

See #13442