Persist FPDFPage_InsertObjectAtIndex() ordering on save [pdfium : main]
Helmut Januschka (Gerrit)
Helmut Januschka added 1 comment![]( "Open in Gerrit")
@the...@chromium.org thanks in advance for the review, let me know if you want me to address anything
Related details
- Lei Zhang
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aryan Krishnan (Gerrit)
Aryan Krishnan added 2 comments![]( "Open in Gerrit")
Hey, Just happened to see this Cl, I have a small nit here.
When inserting before an existing object, adopt that object's content stream so GenerateContent() rewrites the inserted object in place instead of appending a new stream at the end of /Contents.Nit: Commit message needs line wrap :)
Related details
- Helmut Januschka
- Lei Zhang
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lei Zhang (Gerrit)
Lei Zhang added 1 comment![]( "Open in Gerrit")
// Insert a rectangle at index 0; this should be drawn first (underneath theIt would be good to also call `FPDFPage_InsertObjectAtIndex(page.get(), ... 2);` to insert a different colored rectangle that overlaps the 2 pre-existing objects, to also make sure z-ordering works correctly in between.
There's also hello_world_split_streams.pdf, which has multiple streams and can be used to make sure SetContentStream() works correctly and does not confuse multiple existing content streams.
Related details
- Helmut Januschka
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka added 2 comments![]( "Open in Gerrit")
When inserting before an existing object, adopt that object's content stream so GenerateContent() rewrites the inserted object in place instead of appending a new stream at the end of /Contents.Nit: Commit message needs line wrap :)
Done
// Insert a rectangle at index 0; this should be drawn first (underneath theIt would be good to also call `FPDFPage_InsertObjectAtIndex(page.get(), ... 2);` to insert a different colored rectangle that overlaps the 2 pre-existing objects, to also make sure z-ordering works correctly in between.
There's also hello_world_split_streams.pdf, which has multiple streams and can be used to make sure SetContentStream() works correctly and does not confuse multiple existing content streams.
- Aryan Krishnan
- Lei Zhang
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aryan Krishnan (Gerrit)
Aryan Krishnan added 1 comment![]( "Open in Gerrit")
When inserting before an existing object, adopt that object's content stream so GenerateContent() rewrites the inserted object in place instead of appending a new stream at the end of /Contents.Helmut JanuschkaNit: Commit message needs line wrap :)
Done
- Helmut Januschka
- Lei Zhang
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka added 1 comment![]( "Open in Gerrit")
When inserting before an existing object, adopt that object's content stream so GenerateContent() rewrites the inserted object in place instead of appending a new stream at the end of /Contents.Helmut JanuschkaNit: Commit message needs line wrap :)
Aryan KrishnanDone
Quite sure it still needs wrap.
```
Persist FPDFPage_InsertObjectAtIndex() ordering on save
When inserting before an existing object, adopt that object's content
stream so GenerateContent() rewrites the inserted object in place
instead of appending a new stream at the end of /Contents.
Bug: 507508112
Change-Id: I97127e77d698e94f2c55feb66c6847d17011ceab
```
it wraps, at most of 70, am i missing something?
Related details
- Aryan Krishnan
- Lei Zhang
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aryan Krishnan (Gerrit)
Aryan Krishnan added 1 comment![]( "Open in Gerrit")
When inserting before an existing object, adopt that object's content stream so GenerateContent() rewrites the inserted object in place instead of appending a new stream at the end of /Contents.Helmut JanuschkaNit: Commit message needs line wrap :)
Aryan KrishnanDone
Helmut JanuschkaQuite sure it still needs wrap.
```
Persist FPDFPage_InsertObjectAtIndex() ordering on saveWhen inserting before an existing object, adopt that object's content
stream so GenerateContent() rewrites the inserted object in place
instead of appending a new stream at the end of /Contents.Bug: 507508112
Change-Id: I97127e77d698e94f2c55feb66c6847d17011ceab
```it wraps, at most of 70, am i missing something?
On the web editor it seems to extend as far as about 100.
I have added a suggested edit below. Notice how in the old version all of the content remains in line 9 for some reason on gerrit. Applying this should fix it though.
```suggestion
When inserting before an existing object, adopt that object's content
stream so GenerateContent() rewrites the inserted object in place
instead of appending a new stream at the end of /Contents.
- Helmut Januschka
- Lei Zhang
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka added 1 comment![]( "Open in Gerrit")
When inserting before an existing object, adopt that object's content stream so GenerateContent() rewrites the inserted object in place instead of appending a new stream at the end of /Contents.Helmut JanuschkaNit: Commit message needs line wrap :)
Aryan KrishnanDone
Helmut JanuschkaQuite sure it still needs wrap.
Aryan Krishnan```
Persist FPDFPage_InsertObjectAtIndex() ordering on saveWhen inserting before an existing object, adopt that object's content
stream so GenerateContent() rewrites the inserted object in place
instead of appending a new stream at the end of /Contents.Bug: 507508112
Change-Id: I97127e77d698e94f2c55feb66c6847d17011ceab
```it wraps, at most of 70, am i missing something?
On the web editor it seems to extend as far as about 100.
I have added a suggested edit below. Notice how in the old version all of the content remains in line 9 for some reason on gerrit. Applying this should fix it though.
```suggestion
When inserting before an existing object, adopt that object's content
stream so GenerateContent() rewrites the inserted object in place
instead of appending a new stream at the end of /Contents.
```
weird, i might have not have hit "save" in gerrit :) PS3 should be now a copy&paste from your suggestion!, thanks!
Related details
- Aryan Krishnan
- Lei Zhang
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lei Zhang (Gerrit)
Lei Zhang voted Code-Review+1![]( "Open in Gerrit")
- Aryan Krishnan
- Helmut Januschka
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andy Phan (Gerrit)
Andy Phan voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Aryan Krishnan
- Helmut Januschka
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
- Aryan Krishnan
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)
pdfium...@luci-project-accounts.iam.gserviceaccount.com submitted the change![]( "Open in Gerrit")
Change information
Persist FPDFPage_InsertObjectAtIndex() ordering on save
When inserting before an existing object, adopt that object's content
stream so GenerateContent() rewrites the inserted object in place
instead of appending a new stream at the end of /Contents.
- M core/fpdfapi/page/cpdf_pageobjectholder.cpp
- M fpdfsdk/fpdf_edit_embeddertest.cpp
Code-Review: +1 by Andy Phan, +1 by Lei Zhang
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |