Handle empty objects in IsSameTextObject() and free from warning

core/fpdftext/fpdf_text_int.cpp

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "core/fpdftext/fpdf_text_int.h"
 
 #include <algorithm>
 #include <cctype>
@@ skipping 1784 matching lines @@
     }
     if (pTextObj2->GetFontSize() != pTextObj1->GetFontSize()) {
       return FALSE;
     }
   }
   int nPreCount = pTextObj2->CountItems();
   int nCurCount = pTextObj1->CountItems();
   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,
     CPDF_PageObjectList::const_iterator iter) {
   int i = 0;
   while (i < 5 && iter != pObjList->begin()) {
     --iter;
     CPDF_PageObject* pOtherObj = iter->get();
     if (pOtherObj == pTextObj || !pOtherObj->IsText())
       continue;
@@ skipping 667 matching lines @@
   if (!m_bIsParsed || index < 0 || index >= m_LinkList.GetSize()) {
     return;
   }
   CPDF_LinkExt* link = NULL;
   link = m_LinkList.GetAt(index);
   if (!link) {
     return;
   }
   m_pTextPage->GetRectArray(link->m_Start, link->m_Count, rects);
 }