Merge pull request #13691 from calixteman/layout6

XFA - Handle correctly nested containers with lr-tb layout (bug 1718670)

src/core/xfa/html_utils.js

@@ -213,6 +213,7 @@ function layoutText(text, xfaFont, margin, lineHeight, fontFinder, width) {
 function layoutNode(node, availableSpace) {
   let height = null;
   let width = null;
+  let isBroken = false;
 
   if ((!node.w || !node.h) && node.value) {
     let marginH = 0;
@@ -263,6 +264,7 @@ function layoutNode(node, availableSpace) {
       );
       width = res.width;
       height = res.height;
+      isBroken = res.isBroken;
     } else {
       const text = node.value[$text]();
       if (text) {
@@ -276,6 +278,7 @@ function layoutNode(node, availableSpace) {
         );
         width = res.width;
         height = res.height;
+        isBroken = res.isBroken;
       }
     }
 
@@ -287,7 +290,7 @@ function layoutNode(node, availableSpace) {
       height += marginV;
     }
   }
-  return [width, height];
+  return { w: width, h: height, isBroken };
 }
 
 function computeBbox(node, html, availableSpace) {

src/core/xfa/layout.js

@@ -19,6 +19,7 @@ import {
   $getSubformParent,
   $getTemplateRoot,
   $isSplittable,
+  $isThereMoreWidth,
 } from "./xfa_object.js";
 import { measureToString } from "./html_utils.js";
 
@@ -75,6 +76,7 @@ function flushHTML(node) {
 
   node[$extra].children = [];
   delete node[$extra].line;
+  node[$extra].numberInLine = 0;
 
   return html;
 }
@@ -83,9 +85,9 @@ function addHTML(node, html, bbox) {
   const extra = node[$extra];
   const availableSpace = extra.availableSpace;
 
+  const [x, y, w, h] = bbox;
   switch (node.layout) {
     case "position": {
-      const [x, y, w, h] = bbox;
       extra.width = Math.max(extra.width, x + w);
       extra.height = Math.max(extra.height, y + h);
       extra.children.push(html);
@@ -102,16 +104,17 @@ function addHTML(node, html, bbox) {
           children: [],
         };
         extra.children.push(extra.line);
+        extra.numberInLine = 0;
       }
+
+      extra.numberInLine += 1;
       extra.line.children.push(html);
 
       if (extra.attempt === 0) {
         // Add the element on the line
-        const [, , w, h] = bbox;
         extra.currentWidth += w;
         extra.height = Math.max(extra.height, extra.prevHeight + h);
       } else {
-        const [, , w, h] = bbox;
         extra.currentWidth = w;
         extra.prevHeight = extra.height;
         extra.height += h;
@@ -124,7 +127,6 @@ function addHTML(node, html, bbox) {
     case "rl-row":
     case "row": {
       extra.children.push(html);
-      const [, , w, h] = bbox;
       extra.width += w;
       extra.height = Math.max(extra.height, h);
       const height = measureToString(extra.height);
@@ -134,14 +136,12 @@ function addHTML(node, html, bbox) {
       break;
     }
     case "table": {
-      const [, , w, h] = bbox;
       extra.width = Math.min(availableSpace.width, Math.max(extra.width, w));
       extra.height += h;
       extra.children.push(html);
       break;
     }
     case "tb": {
-      const [, , , h] = bbox;
       extra.width = availableSpace.width;
       extra.height += h;
       extra.children.push(html);
@@ -265,6 +265,7 @@ function checkDimensions(node, space) {
     return true;
   }
 
+  const ERROR = 2;
   const parent = node[$getSubformParent]();
   const attempt = (parent[$extra] && parent[$extra].attempt) || 0;
   let y, w, h;
@@ -274,17 +275,19 @@ function checkDimensions(node, space) {
       if (node.w !== "" || node.h !== "") {
         [, , w, h] = getTransformedBBox(node);
       }
+
       if (attempt === 0) {
         // Try to put an element in the line.
 
         if (!node[$getTemplateRoot]()[$extra].noLayoutFailure) {
-          if (node.h !== "" && Math.round(h - space.height) > 1) {
+          if (node.h !== "" && Math.round(h - space.height) > ERROR) {
             // Not enough height.
             return false;
           }
+
           if (node.w !== "") {
             // True if width is enough.
-            return Math.round(w - space.width) <= 1;
+            return Math.round(w - space.width) <= ERROR;
           }
 
           return space.width > 0;
@@ -295,7 +298,7 @@ function checkDimensions(node, space) {
         // Put the element on the line but we can fail
         // and then in the second step (next line) we'll accept.
         if (node.w !== "") {
-          return Math.round(w - space.width) <= 1;
+          return Math.round(w - space.width) <= ERROR;
         }
 
         return space.width > 0;
@@ -308,9 +311,16 @@ function checkDimensions(node, space) {
         return true;
       }
 
-      if (node.h !== "") {
+      if (node.h !== "" && Math.round(h - space.height) > ERROR) {
-        // True if height is enough.
+        return false;
-        return Math.round(h - space.height) <= 1;
+      }
+
+      if (node.w === "" || Math.round(w - space.width) <= ERROR) {
+        return space.height > 0;
+      }
+
+      if (parent[$isThereMoreWidth]()) {
+        return false;
       }
 
       return space.height > 0;
@@ -325,10 +335,19 @@ function checkDimensions(node, space) {
       // is breakable then we can return true.
       if (node.h !== "" && !node[$isSplittable]()) {
         [, , , h] = getTransformedBBox(node);
-        return Math.round(h - space.height) <= 1;
+        return Math.round(h - space.height) <= ERROR;
       }
       // 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) {
+        return space.height > 0;
+      }
+
+      if (parent[$isThereMoreWidth]()) {
+        return false;
+      }
+
       return space.height > 0;
     case "position":
       if (node[$getTemplateRoot]()[$extra].noLayoutFailure) {
@@ -336,7 +355,7 @@ function checkDimensions(node, space) {
       }
 
       [, y, , h] = getTransformedBBox(node);
-      if (node.h === "" || Math.round(h + y - space.height) <= 1) {
+      if (node.h === "" || Math.round(h + y - space.height) <= ERROR) {
         return true;
       }
 
@@ -350,7 +369,7 @@ function checkDimensions(node, space) {
 
       if (node.h !== "") {
         [, , , h] = getTransformedBBox(node);
-        return Math.round(h - space.height) <= 1;
+        return Math.round(h - space.height) <= ERROR;
       }
       return true;
     default:

src/core/xfa/template.js

@@ -39,6 +39,7 @@ import {
   $isBindable,
   $isCDATAXml,
   $isSplittable,
+  $isThereMoreWidth,
   $isTransparent,
   $isUsable,
   $namespaceId,
@@ -948,7 +949,7 @@ class Caption extends XFAObject {
 
     const savedReserve = this.reserve;
     if (this.reserve <= 0) {
-      const [w, h] = this[$getExtra](availableSpace);
+      const { w, h } = this[$getExtra](availableSpace);
       switch (this.placement) {
         case "left":
         case "right":
@@ -1612,8 +1613,18 @@ class Draw extends XFAObject {
     // then we can guess it in laying out the text.
     const savedW = this.w;
     const savedH = this.h;
-    const [w, h] = layoutNode(this, availableSpace);
+    const { w, h, isBroken } = layoutNode(this, availableSpace);
     if (w && this.w === "") {
+      // If the parent layout is lr-tb with a w=100 and we already have a child
+      // which takes 90 on the current line.
+      // If we have a text with a length (in px) equal to 100 then it'll be
+      // splitted into almost 10 chunks: so it won't be nice.
+      // So if we've potentially more width to provide in some parent containers
+      // let's increase it to give a chance to have a better rendering.
+      if (isBroken && this[$getSubformParent]()[$isThereMoreWidth]()) {
+        return HTMLResult.FAILURE;
+      }
+
       this.w = w;
     }
     if (h && this.h === "") {
@@ -2114,10 +2125,20 @@ class ExclGroup extends XFAObject {
     }
   }
 
+  [$isThereMoreWidth]() {
+    return (
+      (this.layout.endsWith("-tb") &&
+        this[$extra].attempt === 0 &&
+        this[$extra].numberInLine > 0) ||
+      this[$getParent]()[$isThereMoreWidth]()
+    );
+  }
+
   [$isSplittable]() {
     // We cannot cache the result here because the contentArea
     // can change.
-    if (!this[$getSubformParent]()[$isSplittable]()) {
+    const parent = this[$getSubformParent]();
+    if (!parent[$isSplittable]()) {
       return false;
     }
 
@@ -2130,6 +2151,15 @@ class ExclGroup extends XFAObject {
       return false;
     }
 
+    if (
+      parent.layout &&
+      parent.layout.endsWith("-tb") &&
+      parent[$extra].numberInLine !== 0
+    ) {
+      // See comment in Subform::[$isSplittable] for an explanation.
+      return false;
+    }
+
     this[$extra]._isSplittable = true;
     return true;
   }
@@ -2174,7 +2204,11 @@ class ExclGroup extends XFAObject {
       children,
       attributes,
       attempt: 0,
-      availableSpace,
+      numberInLine: 0,
+      availableSpace: {
+        width: Math.min(this.w || Infinity, availableSpace.width),
+        height: Math.min(this.h || Infinity, availableSpace.height),
+      },
       width: 0,
       height: 0,
       prevHeight: 0,
@@ -2232,13 +2266,17 @@ class ExclGroup extends XFAObject {
       attributes.xfaName = this.name;
     }
 
-    let failure;
+    const maxRun =
-    if (this.layout === "lr-tb" || this.layout === "rl-tb") {
+      this.layout === "lr-tb" || this.layout === "rl-tb"
-      for (
+        ? MAX_ATTEMPTS_FOR_LRTB_LAYOUT
-        ;
+        : 1;
-        this[$extra].attempt < MAX_ATTEMPTS_FOR_LRTB_LAYOUT;
+    for (; this[$extra].attempt < maxRun; this[$extra].attempt++) {
-        this[$extra].attempt++
+      if (this[$extra].attempt === MAX_ATTEMPTS_FOR_LRTB_LAYOUT - 1) {
-      ) {
+        // If the layout is lr-tb then having attempt equals to
+        // MAX_ATTEMPTS_FOR_LRTB_LAYOUT-1 means that we're trying to layout
+        // on the next line so this on is empty.
+        this[$extra].numberInLine = 0;
+      }
       const result = this[$childrenToHTML]({
         filter,
         include: true,
@@ -2251,24 +2289,12 @@ class ExclGroup extends XFAObject {
       }
     }
 
-      failure = this[$extra].attempt === MAX_ATTEMPTS_FOR_LRTB_LAYOUT;
-    } else {
-      const result = this[$childrenToHTML]({
-        filter,
-        include: true,
-      });
-      failure = !result.success;
-      if (failure && result.isBreak()) {
-        return result;
-      }
-    }
-
     if (!isSplittable) {
       unsetFirstUnsplittable(this);
     }
 
-    if (failure) {
+    if (this[$extra].attempt === maxRun) {
-      if (this[$isSplittable]()) {
+      if (!isSplittable) {
         delete this[$extra];
       }
       return HTMLResult.FAILURE;
@@ -2475,7 +2501,15 @@ class Field extends XFAObject {
       let height = null;
 
       if (this.caption) {
-        [width, height] = this.caption[$getExtra](availableSpace);
+        const { w, h, isBroken } = this.caption[$getExtra](availableSpace);
+        // See comment in Draw::[$toHTML] to have an explanation
+        // about this line.
+        if (isBroken && this[$getSubformParent]()[$isThereMoreWidth]()) {
+          return HTMLResult.FAILURE;
+        }
+
+        width = w;
+        height = h;
         if (this.ui instanceof CheckButton) {
           switch (this.caption.placement) {
             case "left":
@@ -4391,6 +4425,15 @@ class Subform extends XFAObject {
     return true;
   }
 
+  [$isThereMoreWidth]() {
+    return (
+      (this.layout.endsWith("-tb") &&
+        this[$extra].attempt === 0 &&
+        this[$extra].numberInLine > 0) ||
+      this[$getParent]()[$isThereMoreWidth]()
+    );
+  }
+
   *[$getContainedChildren]() {
     // This function is overriden in order to fake that subforms under
     // this set are in fact under parent subform.
@@ -4412,7 +4455,8 @@ class Subform extends XFAObject {
   [$isSplittable]() {
     // We cannot cache the result here because the contentArea
     // can change.
-    if (!this[$getSubformParent]()[$isSplittable]()) {
+    const parent = this[$getSubformParent]();
+    if (!parent[$isSplittable]()) {
       return false;
     }
 
@@ -4436,6 +4480,20 @@ class Subform extends XFAObject {
       return false;
     }
 
+    if (
+      parent.layout &&
+      parent.layout.endsWith("-tb") &&
+      parent[$extra].numberInLine !== 0
+    ) {
+      // If parent can fit in w=100 and there's already an element which takes
+      // 90 then we've 10 for this element. Suppose this element has a tb layout
+      // and 5 elements have a width of 7 and the 6th has a width of 20:
+      // then this element (and all its content) must move on the next line.
+      // If this element is splittable then the first 5 children will stay
+      // at the end of the line: we don't want that.
+      return false;
+    }
+
     this[$extra]._isSplittable = true;
 
     return true;
@@ -4526,7 +4584,11 @@ class Subform extends XFAObject {
       children,
       attributes,
       attempt: 0,
-      availableSpace,
+      numberInLine: 0,
+      availableSpace: {
+        width: Math.min(this.w || Infinity, availableSpace.width),
+        height: Math.min(this.h || Infinity, availableSpace.height),
+      },
       width: 0,
       height: 0,
       prevHeight: 0,
@@ -4600,6 +4662,12 @@ class Subform extends XFAObject {
         ? MAX_ATTEMPTS_FOR_LRTB_LAYOUT
         : 1;
     for (; this[$extra].attempt < maxRun; this[$extra].attempt++) {
+      if (this[$extra].attempt === MAX_ATTEMPTS_FOR_LRTB_LAYOUT - 1) {
+        // If the layout is lr-tb then having attempt equals to
+        // MAX_ATTEMPTS_FOR_LRTB_LAYOUT-1 means that we're trying to layout
+        // on the next line so this on is empty.
+        this[$extra].numberInLine = 0;
+      }
       const result = this[$childrenToHTML]({
         filter,
         include: true,

src/core/xfa/text.js

@@ -223,6 +223,7 @@ class TextMeasure {
       height = 0,
       currentLineWidth = 0,
       currentLineHeight = 0;
+    let isBroken = false;
 
     for (let i = 0, ii = this.glyphs.length; i < ii; i++) {
       const [glyphWidth, glyphHeight, isSpace, isEOL] = this.glyphs[i];
@@ -245,6 +246,7 @@ class TextMeasure {
           currentLineHeight = glyphHeight;
           lastSpacePos = -1;
           lastSpaceWidth = 0;
+          isBroken = true;
         } else {
           currentLineHeight = Math.max(glyphHeight, currentLineHeight);
           lastSpaceWidth = currentLineWidth;
@@ -269,6 +271,8 @@ class TextMeasure {
           width = Math.max(width, currentLineWidth);
           currentLineWidth = glyphWidth;
         }
+        isBroken = true;
+
         continue;
       }
 
@@ -279,7 +283,7 @@ class TextMeasure {
     width = Math.max(width, currentLineWidth);
     height += currentLineHeight + this.extraHeight;
 
-    return { width: WIDTH_FACTOR * width, height };
+    return { width: WIDTH_FACTOR * width, height, isBroken };
   }
 }
 

src/core/xfa/xfa_object.js

@@ -62,6 +62,7 @@ const $isBindable = Symbol();
 const $isDataValue = Symbol();
 const $isDescendent = Symbol();
 const $isSplittable = Symbol();
+const $isThereMoreWidth = Symbol();
 const $isTransparent = Symbol();
 const $isUsable = Symbol();
 const $lastAttribute = Symbol();
@@ -185,6 +186,16 @@ class XFAObject {
     return false;
   }
 
+  /**
+     Return true if this node (typically a container)
+     can provide more width during layout.
+     The goal is to help to know what a descendant must
+     do in case of horizontal overflow.
+   */
+  [$isThereMoreWidth]() {
+    return false;
+  }
+
   [$appendChild](child) {
     child[_parent] = this;
     this[_children].push(child);
@@ -1074,6 +1085,7 @@ export {
   $isDataValue,
   $isDescendent,
   $isSplittable,
+  $isThereMoreWidth,
   $isTransparent,
   $isUsable,
   $namespaceId,

test/pdfs/xfa_bug1718670_1.pdf.link

@@ -0,0 +1 @@
+https://bugzilla.mozilla.org/attachment.cgi?id=9229317

test/test_manifest.json

@@ -952,6 +952,14 @@
        "enableXfa": true,
        "type": "eq"
     },
+    {  "id": "xfa_bug1718670_1",
+       "file": "pdfs/xfa_bug1718670_1.pdf",
+       "md5": "06745be56a89acd80e5bdeddabb7cb7b",
+       "link": true,
+       "rounds": 1,
+       "enableXfa": true,
+       "type": "eq"
+    },
     {  "id": "xfa_bug1718521_1",
        "file": "pdfs/xfa_bug1718521_1.pdf",
        "md5": "9b89dd9e6a4c6c3258ca24debd806863",