Add support for lazy merging dictionaries in annotations #12314
Conversation
There was a problem hiding this comment.
@Snuffleupagus Do you perhaps have time to review this patch since you looked into the dictionary primitive before? I have the feeling that the functionality may be useful, but I'm unsure about the implementation.
| @@ -91,11 +91,11 @@ var Dict = (function DictClosure() { | |||
|
|
|||
| // automatically dereferences Ref objects | |||
| get(key1, key2, key3) { | |||
| let value = this._map[key1]; | |||
There was a problem hiding this comment.
I believe these primitives are called a lot, so introducing extra function calls here might cause a performance regression. If we do this, it will have to be benchmarked carefully.
There was a problem hiding this comment.
Agreed.
My first try was to use a Proxy object which should be fine to avoid any kind of perf regression but I didn't find a polyfill for it (according to MDN it isn't supported with IE) and so I gave up this solution.
But I guess that it should be possible now to use this solution since IE support has been removed.
Wdyt ?
There was a problem hiding this comment.
I think we should aim for a much simpler solution, specifically for the case at hand here, rather than trying to extend a primitive such as Dict since there's other problems with this patch as far as I can tell.
E.g. the getLazyMergeDict function is mutating the provided dict parameter, which could very easily lead to issues elsewhere and even outright bugs if used without the out-most care (since you'd not expect that kind of behaviour).
Since I hack on PDF.js in my spare time, I've not yet had the time to look into all of this, but hopefully will later this week.
|
Closing since this is replaced by #12361. |
It aims to fix for example issue #12294.
The merge have to be lazy to avoid to write too much potentially useless data when saving a form.