XFA - Handle correctly nested containers with lr-tb layout (bug 1718670) #13691
Conversation
calixteman
commented
Jul 7, 2021
•
calixteman
commented
- and avoid to push a field with no dimensions when we have some available space in width in a parent.
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.67.70.0:8877/392ff196b68a244/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://3.101.106.178:8877/487370c898ba584/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/392ff196b68a244/output.txt Total script time: 31.97 mins
Image differences available at: http://54.67.70.0:8877/392ff196b68a244/reftest-analyzer.html#web=eq.log |
|
There is a regression in |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/487370c898ba584/output.txt Total script time: 36.07 mins
Image differences available at: http://3.101.106.178:8877/487370c898ba584/reftest-analyzer.html#web=eq.log |
|
I'm sad here because I'd say that this "yellow pages" on 2 lines is not so bad... |
|
I'm OK with accepting the regression, @brendandahl wdyt? |
|
I think it's better to fix: we could have strange things because of that. |
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.67.70.0:8877/243e169c2dc179f/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://3.101.106.178:8877/38387c3b1bd87c6/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/243e169c2dc179f/output.txt Total script time: 31.86 mins
Image differences available at: http://54.67.70.0:8877/243e169c2dc179f/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/38387c3b1bd87c6/output.txt Total script time: 36.10 mins
Image differences available at: http://3.101.106.178:8877/38387c3b1bd87c6/reftest-analyzer.html#web=eq.log |
- and avoid to push a field with no dimensions when we have some available space in width in a parent.
|
/botio makeref |
From: Bot.io (Linux m4)ReceivedCommand cmd_makeref from @calixteman received. Current queue size: 0 Live output at: http://54.67.70.0:8877/69db92dd5b27f90/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_makeref from @calixteman received. Current queue size: 1 Live output at: http://3.101.106.178:8877/58f7e48451a05be/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/69db92dd5b27f90/output.txt Total script time: 28.68 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/58f7e48451a05be/output.txt Total script time: 33.56 mins
|
| } | ||
| // Else wait and see: this node will be layed out itself | ||
| // in the provided space and maybe a children won't fit. | ||
|
|
||
| if (node.w === "" || Math.round(w - space.width) <= ERROR) { |
There was a problem hiding this comment.
@calixteman It seems that LGTM complains about this line, since w === undefined always holds here; which I'm guessing is probably not what you actually want?
Could we simplify all of the code in the various cases by replacing
Line 271 in c33bf0b
with unconditionally doing
const [, y, w, h] = getTransformedBBox(node); instead?
If so, I can submit a patch to fix that!
… (PR 13691 follow-up) This way we ensure that these BBox values are *always* defined as expected for every `case`-block, and we also don't need to duplicate the lookup in multiple places. (Also, the patch removes a couple of unnecessary line-breaks in existing comments.) Fixes mozilla#13691 (review), which was flagged by LGTM.
… (PR 13691 follow-up) This way we ensure that these BBox values are *always* defined as expected for every `case`-block, and we also don't need to duplicate the lookup in multiple places. (Also, the patch removes a couple of unnecessary line-breaks in existing comments.) Fixes mozilla#13691 (review), which was flagged by LGTM.
