Skip to content

TIKA-4831: The icon of a GeoGebra tool is its thumbnail - #3100

Merged
THausherr merged 3 commits into
apache:mainfrom
dschmidt:geogebra-tool-icon
Aug 31, 2026
Merged

THausherr merged 3 commits into
apache:mainfrom
dschmidt:geogebra-tool-icon

Conversation

@dschmidt

Copy link
Copy Markdown
Contributor

Follow-up to #3044: a tool file (.ggt) has no geogebra_thumbnail.png, but a tool can have an icon, stored in a directory with a generated name and referenced by the macro's iconFile attribute (File Format reference, ".ggt"). The parser now emits that icon as the tool's THUMBNAIL, and not a second time as a picture.

Order of preference is unchanged for the other files: the worksheet thumbnail, then the first slide thumbnail, then the tool icon. A worksheet with an embedded tool keeps its own thumbnail; the icon stays INLINE there.

I could not find a real .ggt with an icon anywhere public (GitHub does not index binaries, GeoGebra's site offers no tools for download), so the test builds one after the XML reference.

https://issues.apache.org/jira/browse/TIKA-4831

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the GeoGebra parsing logic so that a GeoGebra tool’s icon (referenced via the macro’s iconFile inside a .ggt) is treated as the document thumbnail (embeddedResourceType=THUMBNAIL) and is not emitted again as a normal picture, aligning Tika output with how GeoGebra represents tools.

Changes:

  • Collect iconFile values from macros while parsing geogebra_macro.xml.
  • Extend thumbnail selection to fall back to the first existing tool icon when no worksheet/slide thumbnail is available, and avoid duplicate emission of that same entry.
  • Add unit tests that synthesize a .ggt/worksheet-with-macro to verify THUMBNAIL vs INLINE behavior, and document the change in CHANGES.txt.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-miscoffice-module/src/test/java/org/apache/tika/parser/geogebra/GeoGebraParserTest.java Adds tests covering tool-icon-as-thumbnail and worksheet-thumbnail precedence over embedded tool icon.
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-miscoffice-module/src/main/java/org/apache/tika/parser/geogebra/GeoGebraXMLHandler.java Records macro iconFile paths during SAX parsing for later thumbnail selection.
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-miscoffice-module/src/main/java/org/apache/tika/parser/geogebra/GeoGebraParser.java Updates thumbnail selection order to include tool icons and prevents double-emitting the chosen thumbnail entry.
CHANGES.txt Notes the GeoGebra tool icon THUMBNAIL behavior change for the upcoming release.
Suppressed comments (1)

tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-miscoffice-module/src/main/java/org/apache/tika/parser/geogebra/GeoGebraParser.java:388

  • The inline comment here is too specific: this skip applies to whatever entry was emitted as the representative thumbnail (worksheet thumbnail, slide thumbnail, or tool icon), not only a tool icon. Updating the comment will prevent confusion when reading the loop logic later.
            if (name.equals(thumbnail)) {
                //already emitted as the thumbnail (a tool icon)
                continue;
            }

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 303 to +310
/**
* Parses one GeoGebra XML for its text and, if {@code documentMetadata}
* is set, the document metadata. A part that cannot be read or is not
* well-formed is recorded in the metadata and skipped.
*/
private void parseGeoGebraXml(ZipFile zipFile, ZipArchiveEntry entry,
XHTMLContentHandler xhtml, Metadata metadata,
boolean documentMetadata, ParseContext context)
/**
* @return the icon files of the macros in the XML, in document order
*/

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged into one.

@THausherr
THausherr merged commit 19ca284 into apache:main Aug 31, 2026
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants