| Summary: | FILEOPEN pptx: Master slide texts show with specific placeholder type | ||
|---|---|---|---|
| Product: | LibreOffice | Reporter: | Korrawit Pruegsanusak <detective.conan.1412> |
| Component: | Impress | Assignee: | Korrawit Pruegsanusak <detective.conan.1412> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | muthu.subramanian.karunanidhi |
| Priority: | medium | ||
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | target:4.3.0 target:4.1.5 target:4.2.0.0.beta2 | ||
| Crash report or crash signature: | Regression By: | ||
| Attachments: |
pptx file
screenshot |
||
|
Description
Korrawit Pruegsanusak
2013-11-09 18:53:00 UTC
Created attachment 88943 [details]
screenshot
Hello Muthu, Since you have fixed bug 35372 with this commit http://cgit.freedesktop.org/libreoffice/core/commit/?id=90352e5f1e50291960c944f9a1f44ab3e91d6503 Source code: http://opengrok.libreoffice.org/xref/core/oox/source/ppt/pptgraphicshapecontext.cxx#147 There are 2 choices for fixing this problem: 1. add a check for XML_body in this line (line 151) > if( pPlaceholder->getSubType() == XML_obj ) 2. do as your TODO said (line 150) > TODO: Check if pPlaceholder->getSubType is none (i.e. none explicitly specified) That is, should I add a guard for all possible types of placeholder? Muthu, I'd like to ask that, which should I do? Oops, forgot to CC Muthu :\ Muthu, could you please read comment 2, thanks. Submitted patch for review at https://gerrit.libreoffice.org/6814 I selected the second choice because I've found pptx document that has an image with placeholder type of "title". Just saw this one :) So, the patch seems fine. Thank you! (But, I am not too sure about that ToDo. Could be some older one?) Korrawit Pruegsanusak committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=bb1213a740b3b3b2b1967639939dd7a72c6d4237 fdo#71434: don't show master text if PlaceHolder types defined The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. But, the ToDo seems to refer to: "what happens if SubType isn't set"...anyways... Hello Muthu, Thanks for review & push :) (In reply to comment #7) > But, the ToDo seems to refer to: "what happens if SubType isn't > set"...anyways... Oh, so I misunderstood your TODO. But I don't think this will change an outcome, because bUseText still be true (since initialization) in both old and new code. Anyway, I submitted another 2 patches for stable branches: -4-2 at https://gerrit.libreoffice.org/6822 -4-1 at https://gerrit.libreoffice.org/6821 Could you please review & push again? Thanks :) It would be nice if somebody else reviews that for the stable branches :) Korrawit Pruegsanusak committed a patch related to this issue. It has been pushed to "libreoffice-4-1": http://cgit.freedesktop.org/libreoffice/core/commit/?id=0f4a452be3342d0206d6724faf170fa2786a7777&h=libreoffice-4-1 fdo#71434: don't show master text if PlaceHolder types defined It will be available in LibreOffice 4.1.5. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. Korrawit Pruegsanusak committed a patch related to this issue. It has been pushed to "libreoffice-4-2": http://cgit.freedesktop.org/libreoffice/core/commit/?id=a06bc4102934c47e1aebe53ea24415fafb7c506a&h=libreoffice-4-2 fdo#71434: don't show master text if PlaceHolder types defined It will be available in LibreOffice 4.2. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. Thanks very much, resolving. Anyway, while creating the unit test, I encountered another bug, reported at bug#72260. It would be great if you could take a look there. Thanks again! :) Korrawit Pruegsanusak committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=6fd352dc3ad32078188537aa8849758735391f46 unittest for fdo#71434 The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. |