Add option to delete XMP metadata by vithlaithla · Pull Request #8681 · JabRef/jabref

vithlaithla · 2022-04-17T11:34:54Z

Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Hi, I am working with Jefferson on this issue and we made some changes to fix issue #8277.
Our changes added a button to delete metadata that are selected under xmpPreferences in the screenshot here.

We also added a checkbox to include all fields inside the xmpPreferences, which should select all fields of an entry.The UI works as follows:

To enable the "Select All Fields" checkbox, the "Do not write the following fields to XMP metadata" must be checked.
The checkbox, when enabled, disable the filter list.

There are some problems that we face:

When we make changes to the metadata, sometimes it corrupts the file's metadata. For our XmpUtilRemover, we mainly reuse the XmpUtilWriter, so we might be misusing some functionalities.
We are not sure on how to test the functions yet. We hope to get some guidance.
We changed the xmpPreferences's class, it might be a disruptive change.

… currently the button uses the Preferences's xmp Privacy Filter to delete. In deletion, the functionality mirrors XmpUtilWriter except that it still lacks DublinCore settings and it does not set the other fields to whatever values the filter does not include.

disable filter list if select-all is checked change to XmpPreference class to add field for select-all

Siedlerchr · 2022-04-17T12:10:45Z

Thanks for your contribution, please setup Jabrefs code style for intellij https://github.com/JabRef/jabref/tree/main/config

ThiloteE · 2022-04-17T13:59:02Z

When we make changes to the metadata, sometimes it corrupts the file's metadata.

What do you mean by that? Can you give an example?

Up until 2022-04-11, the development versions of JabRef were reproducibly corrupting pdf files. There was an issue with pdfbox version 3, with was (hopefully!) fixed by Fix XMP write #8658. Corrupt pdf files BEFORE that date can be explained by 8658. Please make sure to retest with the latest commits from AFTER that date.
We are not sure on how to test the functions yet.

You could try to create an entry with all supported fields. Write XMP metadata to PDF. Check with exiftool if metadata was written. Then, try to remove the metadata. Check with exiftool, if it was removed. See Add feature that removes XMP metadata from pdf(s) #8277 (comment) for info about exiftool.

This method might be inefficient and is prone to user error. Maybe there are other ways how this could be tested, but if push comes to shove...
This is above my head.

koppor · 2022-04-18T10:47:58Z

Regarding the code style warnings. You can go to the "Files" tab in this PR and see the warnings directly. Deep link: https://github.com/JabRef/jabref/pull/8681/files

Screenshot:

May we ask that you DO NOT reformat the JavaDoc comments when you do not touch them? Reformatting then to a single line makes them more hard to read.

The previous commit formatted the javadoc This reverts commit ced851f.

koppor

A short review with some comments. More comments will follow, but you can work on the current comments if you want!

koppor · 2022-04-18T10:51:13Z

+                                optionsDialog.getProgressArea().appendText("  " + Localization.lang("Error while deleting") + " '"
+                                        + file.toString() + "':\n");


Localization works in a different way. See https://devdocs.jabref.org/getting-into-the-code/code-howtos#using-localization-correctly for details.

In short: Use placeholders instead of string concatenation.

Currently, we log the exception (Logger.error(...)), but we do not show the concrete error in the UI.

(The developers should refine https://devdocs.jabref.org/getting-into-the-code/code-howtos#outputting-errors-in-the-ui accordingly).

koppor · 2022-04-18T11:12:41Z

+                Platform.runLater(() -> optionsDialog.getProgressArea()
+                                                     .appendText("  " + Localization.lang("Skipped - No PDF linked") + ".\n"));


Isn't it possible to collect the messages using a StringJoiner and then output it as a whole?

Yes, please see the changes.

koppor · 2022-04-18T11:13:56Z

+            }
+        }
+
+        errors = entriesChanged = skipped = 0;


Please split up into multiple lines - we do not have the joined assignements as code style.

koppor · 2022-04-18T11:14:43Z

+        if (optionsDialog == null) {
+            optionsDialog = new OptionsDialog();
+        }


Shouldn't the options dialog be recreated at each run? To ensure that the information messages are empty at each run? If not, please comment here why caching is a good idea

I took your suggestion and recreated at each run.

koppor · 2022-04-18T11:45:30Z

+
+    private void DeleteMetadataFromFile(Path file, BibEntry entry, BibDatabaseContext databaseContext, BibDatabase database) throws Exception {
+        XmpUtilRemover.deleteXmp(file, entry, database, xmpPreferences);
+        embeddedBibExporter.exportToFileByPath(databaseContext, database, filePreferences, file);


I don't know what this does? export does not read as delete. (Don't have the code at hand currently).

The export, the second line, is removed and it still have its intended function

…to same file, also fix some small text

consider select-all-fields when filling dublin core schema rename test file

…rtSingleEntryWithPrivacyFilter test method in XmpExporterTest class

…-for-issue-8277

vithlaithla · 2022-04-19T16:43:59Z

@ThiloteE

Thanks for mentioning that, we look into the PR you mentioned and it had the changes to resolve said problem.
We wrote some test cases for the function, so hopefully those should cover most use cases. Since there were in-built functions, we used them to craft our test cases.

…line to fix

vithlaithla · 2022-04-19T16:59:32Z

For the changes:

we refine the initially added functions by following @koppor's requests and some changes ourselves,
resolves the first problem we faced by following Fix XMP write #8658 that @ThiloteE mentioned,
added tests to make it work as intended

ThiloteE · 2022-04-19T17:05:22Z

You are welcome! I am impressed with the amount of changes that needed to be done for this feature. Thank you! Will try out this pull request within the next 3 days.

vithlaithla · 2022-04-21T13:23:03Z

@koppor I fixed the check style and it passed the check style test. However, the fetcher test failed here but passed on my fix branch 'revise-8277' on my repository. Which, I then merged to this branch. I read the log, and there seems to be a network problem if I'm not mistaken. Can I do something here?

ThiloteE · 2022-04-21T13:43:03Z

I just did some testing:

Test A)

Have
choose

Result: No metadata got deleted.
Comment: I think this is counterintuitive, as users explicitly press a button that says "delete metadata".
- Suggestion A) Either delete all (JabRef supported) metadata fields in this case.
- Suggestion B) OR change the button to read "Delete metadata from PDFs (as specified in preferences)" or something similar.
This is a non-critical remark and could also be adressed in a follow up pull request.

Test B)

Have following entry:

@Article{test444differentfilet,
author = {test444differentfile},
title  = {title444},
file   = {:test 444 (2021).pdf:PDF},
}

Have following preferences:
Press: Tools > Delete metadata from PDFs

Result:

ThiloteE · 2022-04-21T14:29:17Z

Problem: Selecting the field "Do not write the following fields to XMP metadata" will turn out to be ambiguous to the user. It will fulfill three separate functions:

Fields from entry will not be writtten to XMP metadata
Existing metadata will be removed from the PDFs, IF the entry does have the same field as specified in the preferences included in the entry. (I know, this sounds complicated. How to explain to users?).
ALL existing JabRef supported metadata fields will be removed from the PDFs, IF the option select all fields is selected.

This seems a lot for one single option and will lead to confusion. At the very least, there needs to be some kind of documentation. Maybe this can be separated into separate tickable preference?

Another option would be renaming. How about: "Prevent writing following metadata fields to PDFs / Remove following metadata fields from PDFs"?

Underneath the option, put following text:

      - Choosing this option and using "write metadata to PDF files" will not write the selected XMP metadata fields to PDFs. 
        Additionally, it will also remove existing metadata from PDFs, if one or multiple of the following fields
        are ALSO present in the entry, whose metadata is supposed to be written to PDF.
      - Selected XMP metadata fields will be completely removed from PDFs if choosing "Delete metadata from PDFs".

ThiloteE · 2022-04-21T14:30:37Z

I would be happy, if you could address my remarks!

JeffersonJefferson-pixel · 2022-04-21T18:01:42Z

@ThiloteE
Regarding the unsuccessful metadata removal, we suspect the the reason that only author and title field are deleted is because they are included in the entry.

So when deleting, we don't need to consider any information about the library data of entry (whether fields to be deleted are also in the entry...). Is that right?

Regarding ambiguity, we are leaning more toward the second option (Clearer description) because it seems simpler for us.

…ta of entry some changes to interface in XmpUtilRemover some changes to JavaDoc some changes to test add description to test add text description to privacy filter tab

koppor · 2022-04-21T19:16:24Z

Suggestion A) Either delete all (JabRef supported) metadata fields in this case.

+1 for that. - I think, the intention of the issue is to clean-up the meta data of a PDF (as you shown in the screenshots).

The preference setting was more a privacy setting - maybe, we should remove that setting in the future. --> @ThiloteE Let's discuss the use cases and workflows soon.

ThiloteE · 2022-04-21T23:28:18Z

So when deleting, we don't need to consider any information about the library data of entry (whether fields to be deleted are also in the entry...). Is that right?

When pressing the button Tools > Delete metadata from PDFs, it does NOT need to be considered. (But one should be able to choose which fields are supposed to be deleted.)
When pressing the button Tools > Write metadata to PDF files fields within the entry SHOULD be considered.
- Maybe we can drop existing fields being deleted when pressing this button, though. Why delete fields, when we press a button that says "write metadata ..."?, when we now we have the button "delete metadata from PDFs" instead, right? So this functionality is maybe not necessary anymore.

Ideally, I think the following (or something similar) would be the least confusing solution:

But I know you have done a lot! Feel free to do whatever you think will work and whatever you think you may manage in your given time or with the knowledge you possess! :-)

JeffersonJefferson-pixel · 2022-04-22T13:14:26Z

@ThiloteE
In this change, we now don't consider whether fields are also in entries when deleting.
Demo:

Have following entry
@Misc{, author = {Jefferson}, year = {2022}, file = {:JabRef_multipleMetaEntries.pdf:PDF}, }
Have following preferences

PDF file's initial XMP metadata

linked file view:

exiftool result:

Press: Tools > Delete metadata from PDFs
PDF file's XMP metadata after deleting

linked file view:

exiftool result:

Regarding ambiguity, we added the description:

Problem/Confusion we have:

Linked file view model's XMP metadata column is empty and it says "Could not extract Metadata from XMP metadata".
This is probably because all XMP metadata have been deleted. We are not sure if this behavior is desirable.
We have difficulty passing the fetcher test. Do you have any suggestion for this problem?

ThiloteE · 2022-04-22T13:30:28Z

I will try it. What do I have to do to start "Linked file view model"? E.g. how can I find the dialogue for what you show in the picture beneath 5.?

JeffersonJefferson-pixel · 2022-04-22T13:42:57Z

@ThiloteE
Here is how you can access it

ThiloteE · 2022-04-23T13:10:31Z

Test C:

BEFORE:

I then clicked on Tools > Write metadata to PDF files (F6)

AFTER:

Exiftool AFTER:

ThiloteE · 2022-04-23T13:27:57Z

Test D:
I went to get another fresh PDF (same data was attached to the PDF as in test C BEFORE doing stuff)

I then clicked Tools > Delete metadata from PDFs

AFTER:

exiftool AFTER:

ThiloteE · 2022-04-23T13:38:26Z

Ah, and I had a lot of Typos in my text. It does not really look good.

Here, this is a better text:

Underneath the option, put following (or something similar) text and format it in a way that looks nice! :D

      - Choosing this option and using "write metadata to PDF files" will not write the selected XMP metadata fields to PDFs. 
        Additionally, it will also remove existing metadata from PDFs, if one or multiple of the following fields
        are ALSO present in the entry, whose metadata is supposed to be written to PDF.
      - Selected XMP metadata fields will be completely removed from PDFs if choosing "Delete metadata from PDFs".

To do in a follow up pull-request:

Add this text to the JabRef documentation page and link to it via following button:

But, this can be done later, not now.

ThiloteE · 2022-04-23T13:39:32Z

But looking already pretty good. You are definitely getting closer to the desired outcome!

vithlaithla · 2022-04-24T16:11:53Z

got it, we will follow up very soon

ThiloteE · 2022-05-24T05:08:45Z

what is the status here?

vithlaithla · 2022-05-25T11:41:49Z

@ThiloteE There is a delay on the work, but we are still working on it! We will let you know once we have the changes.

ThiloteE · 2022-05-25T11:43:53Z

ok thanks! :)))

calixtus · 2022-08-14T05:02:43Z

any update here?

ThiloteE · 2022-11-16T11:50:04Z

Attempt at summarizing this PR:

As far as I remember

Done:

1. Deleting XMP metadata mostly WORKS.

To Do:

2. This PR causes a regression and breaks "parse metadata from pdf" in "General" tab of entry editor. See Add option to delete XMP metadata #8681 (comment).
- The regression was caused by one of the latest commits.
- JabRef and Exiftool now show different attached metadata.
- When working on this PR, don't trust JabRef. Exiftool seems to be more reliable.
3. "Linked file view model's XMP metadata column is empty and it says "Could not extract Metadata from XMP metadata".
This is probably because all XMP metadata have been deleted. We are not sure if this behavior is desirable." - See Add option to delete XMP metadata #8681 (comment)

To Do In Follow Up PR:

4. Some exceptions to deleting XMP metadata remain. Some fields cannot be deleted yet. See Add option to delete XMP metadata #8681 (comment)
5. Explanation of preferences looks slightly out of place/ugly. See Add option to delete XMP metadata #8681 (comment)
- If push comes to shove, the explanation could be moved to the docs

calixtus · 2023-04-03T14:56:29Z

Closing this issue due to inactivity 💤
Please reopen if you plan to continue on this PR

vithlaithla and others added 4 commits April 12, 2022 11:08

add checkbox to select all fields in preference for delete purpose

cd595cf

disable filter list if select-all is checked change to XmpPreference class to add field for select-all

fix conflict

4a8b503

add text to pass LocalizationConsistencyTest

0f1157f

vithlaithla marked this pull request as ready for review April 17, 2022 11:35

vithlaithla marked this pull request as draft April 17, 2022 11:37

fixes previously added files' code style to jabref's

ced851f

vithlaithla added 2 commits April 18, 2022 18:16

Revert "fixes previously added files' code style to jabref's"

e676757

The previous commit formatted the javadoc This reverts commit ced851f.

fixes previous commit's (0f1157f) code style

841d0e1

koppor requested changes Apr 18, 2022

View reviewed changes

vithlaithla and others added 10 commits April 18, 2022 22:56

use StringJoiner and fix usage of Localization

35e1b52

create temp file to remove errors of write and read at the same time …

81ab83a

…to same file, also fix some small text

write some initial tests

498675f

added javadoc

d16b4d9

rewrite to dublin core after deleting

4564ad5

consider select-all-fields when filling dublin core schema rename test file

add mock return value of xmpPreferences.getSelectAllfields() for expo…

c85bcef

…rtSingleEntryWithPrivacyFilter test method in XmpExporterTest class

fix localization

76f45ba

remove cache from option dialog and unncessary bib export

713b9ef

added changes to CHANGELOG.md

c7c7697

Merge branch 'main' of https://github.com/vithlaithla/jabref into fix…

17fa29a

…-for-issue-8277

CHANGELOG.md had checkstyle errors around headings, so added a blank …

c6168dc

…line to fix

Siedlerchr changed the title ~~Fix for issue 8277~~ Add option to delete XMP datadata Apr 19, 2022

Siedlerchr changed the title ~~Add option to delete XMP datadata~~ Add option to delete XMP metadata Apr 19, 2022

ThiloteE added the component: external-files label Apr 20, 2022

no longer consider whether fields to be deleted are also in libray da…

c21285f

…ta of entry some changes to interface in XmpUtilRemover some changes to JavaDoc some changes to test add description to test add text description to privacy filter tab

fix localization

9cffa99

ThiloteE mentioned this pull request Apr 24, 2022

Don't write enclosing braces to PDF metadata? #8452

Closed

ThiloteE added the [outdated] type: feature label Apr 30, 2022

ThiloteE removed the [outdated] type: feature label Jul 9, 2022

ThiloteE added the status: changes-required Pull requests that are not yet complete label Nov 18, 2022

calixtus closed this Apr 3, 2023

koppor mentioned this pull request Apr 3, 2023

Add feature that removes XMP metadata from pdf(s) #8277

Closed

ganesh-vk mentioned this pull request Jan 13, 2026

Feature: Add ability to remove XMP metadata from Linked Files #14853

Merged

6 tasks

		optionsDialog.getProgressArea().appendText(" " + Localization.lang("Error while deleting") + " '"
		+ file.toString() + "':\n");

		Platform.runLater(() -> optionsDialog.getProgressArea()
		.appendText(" " + Localization.lang("Skipped - No PDF linked") + ".\n"));

Uh oh!

Conversation

vithlaithla commented Apr 17, 2022 • edited by Siedlerchr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Siedlerchr commented Apr 17, 2022

Uh oh!

ThiloteE commented Apr 17, 2022

Uh oh!

koppor commented Apr 18, 2022

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vithlaithla commented Apr 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vithlaithla commented Apr 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ThiloteE commented Apr 19, 2022

Uh oh!

vithlaithla commented Apr 21, 2022

Uh oh!

ThiloteE commented Apr 21, 2022

Uh oh!

ThiloteE commented Apr 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ThiloteE commented Apr 21, 2022

Uh oh!

JeffersonJefferson-pixel commented Apr 21, 2022

Uh oh!

koppor commented Apr 21, 2022

Uh oh!

ThiloteE commented Apr 21, 2022

Uh oh!

JeffersonJefferson-pixel commented Apr 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ThiloteE commented Apr 22, 2022

Uh oh!

JeffersonJefferson-pixel commented Apr 22, 2022

Uh oh!

ThiloteE commented Apr 23, 2022

Uh oh!

ThiloteE commented Apr 23, 2022

Uh oh!

ThiloteE commented Apr 23, 2022

Uh oh!

ThiloteE commented Apr 23, 2022

Uh oh!

vithlaithla commented Apr 24, 2022

Uh oh!

ThiloteE commented May 24, 2022

Uh oh!

vithlaithla commented May 25, 2022

Uh oh!

ThiloteE commented May 25, 2022

Uh oh!

vithlaithla commented Apr 17, 2022 •

edited by Siedlerchr

Loading

vithlaithla commented Apr 19, 2022 •

edited

Loading

vithlaithla commented Apr 19, 2022 •

edited

Loading

ThiloteE commented Apr 21, 2022 •

edited

Loading

JeffersonJefferson-pixel commented Apr 22, 2022 •

edited

Loading

ThiloteE commented Nov 16, 2022 •

edited

Loading