Handle empty objects in IsSameTextObject() and free from warning

core/fpdftext/fpdf_text_int.cpp

Index: core/fpdftext/fpdf_text_int.cpp
diff --git a/core/fpdftext/fpdf_text_int.cpp b/core/fpdftext/fpdf_text_int.cpp
index be01648d4574d8eee30d702f8bf238d61e860730..85740fe73f67e8d441b0bc473a491734eb521a3a 100644
--- a/core/fpdftext/fpdf_text_int.cpp
+++ b/core/fpdftext/fpdf_text_int.cpp
@@ -1802,25 +1802,30 @@ FX_BOOL CPDF_TextPage::IsSameTextObject(CPDF_TextObject* pTextObj1,
   if (nPreCount != nCurCount) {
     return FALSE;
   }
-  CPDF_TextObjectItem itemPer, itemCur;
+
+  CPDF_TextObjectItem itemPer;
+  CPDF_TextObjectItem itemCur;
   for (int i = 0; i < nPreCount; i++) {
     pTextObj2->GetItemInfo(i, &itemPer);
     pTextObj1->GetItemInfo(i, &itemCur);
     if (itemCur.m_CharCode != itemPer.m_CharCode) {
       return FALSE;
     }
-  }
-  if (FXSYS_fabs(pTextObj1->GetPosX() - pTextObj2->GetPosX()) >

[jun_fang, 2016/03/18 05:31:48]

Code here is used to check whether the distance of two text objects exceeds a
specific range (calculated by char width and font size). If two text objects are
close enough and them have the same content, they are considered as the same
object. Otherwise, they are different objects. Putting code inside of loop may
make readers a little bit confused. I think we should keep it outside of loop.

-          GetCharWidth(itemPer.m_CharCode, pTextObj2->GetFont()) *
-              pTextObj2->GetFontSize() / 1000 * 0.9 ||
-      FXSYS_fabs(pTextObj1->GetPosY() - pTextObj2->GetPosY()) >
-          std::max(std::max(rcPreObj.Height(), rcPreObj.Width()),
-                   pTextObj2->GetFontSize()) /
-              8) {
-    return FALSE;
+    if (i == nPreCount - 1) {

[jun_fang, 2016/03/18 05:31:48]

If I work on this issue, I may do code change like:

CPDF_TextPage::IsSameTextObject(CPDF_TextObject* pTextObj1,
   if (nPreCount != nCurCount) {
     return FALSE;
   }
-  CPDF_TextObjectItem itemPer, itemCur;
+  CPDF_TextObjectItem itemPer = {0, 0.0f, 0.0f};
+  CPDF_TextObjectItem itemCur = {0, 0.0f, 0.0f};
   for (int i = 0; i < nPreCount; i++) {
     pTextObj2->GetItemInfo(i, &itemPer);
     pTextObj1->GetItemInfo(i, &itemCur);
@@ -1810,6 +1811,8 @@ FX_BOOL CPDF_TextPage::IsSameTextObject(CPDF_TextObject*
pTextObj1,
       return FALSE;
     }
   }
+  if (!itemPer.m_CharCode)
+    return TRUE;
   if (FXSYS_fabs(pTextObj1->GetPosX() - pTextObj2->GetPosX()) >
           GetCharWidth(itemPer.m_CharCode, pTextObj2->GetFont()) *
               pTextObj2->GetFontSize() / 1000 * 0.9 ||

[Tom Sepez, 2016/03/18 16:13:43]

Ok, this is close to what Wei originally suggested.  Lets take this, then. 
Thanks for clarifiying.

[Wei Li, 2016/03/18 16:59:44]

Thank you for the clarification and suggestion. I put the condition back outside
loop now. For empty objects, I think it is better to return early based on the
item count since 0 is still a valid charcode.

+      if (FXSYS_fabs(pTextObj1->GetPosX() - pTextObj2->GetPosX()) >
+              GetCharWidth(itemPer.m_CharCode, pTextObj2->GetFont()) *
+                  pTextObj2->GetFontSize() / 1000 * 0.9 ||
+          FXSYS_fabs(pTextObj1->GetPosY() - pTextObj2->GetPosY()) >
+              std::max(std::max(rcPreObj.Height(), rcPreObj.Width()),
+                       pTextObj2->GetFontSize()) /
+                  8) {
+        return FALSE;
+      }
+    }
   }
   return TRUE;
 }
+
 FX_BOOL CPDF_TextPage::IsSameAsPreTextObject(
     CPDF_TextObject* pTextObj,
     const CPDF_PageObjectList* pObjList,