After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 649172 - ImageMap 'open' action should unescape HTML characters
ImageMap 'open' action should unescape HTML characters
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Plugins
2.8.4
Other All
: Normal normal
: 2.8
Assigned To: GIMP Bugs
GIMP Bugs
: 693872 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-05-02 11:23 UTC by Nick Fenwick
Modified: 2013-04-11 08:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Unescape simple xml encodings when reading csim format image maps (2.66 KB, patch)
2013-02-17 23:00 UTC, Martin Husemann
none Details | Review
Replacement for g_markup_escape_text() to unescape HTML characters (3.41 KB, patch)
2013-02-18 12:22 UTC, K Sashi Kumar
none Details | Review
Implementation of unmarkup function to unescape the HTML characters (3.29 KB, patch)
2013-03-04 19:12 UTC, K Sashi Kumar
none Details | Review
Fixes patch in comment #2 by using memmove() (3.32 KB, patch)
2013-04-06 11:45 UTC, Mukund Sivaraman
none Details | Review

Description Nick Fenwick 2011-05-02 11:23:51 UTC
Briefly: ImageMap saves quotes as """, opens them as """ and re-saves """

I'm on Fedora 14 with a stock 2.6.11 gimp.

Create an ImageMap (Filters->Web->ImageMap), create any map shape, and assign it a "onMouseOver" action of "doSomething("hello")";  The intention is to call the javascript doSomething function, passing a string "hello".

If you edit the shape before saving, it's still "doSomething("hello").

Save the ImageMap.  What's saved is e.g.:

<area shape="rect" coords="143,89,170,182" onmouseover="doSomething(&quot;hello&quot;)"  nohref="nohref" />

If you then load and save the ImageMap, with no other changes, what's saved is:

<area shape="rect" coords="143,89,170,182" onmouseover="doSomething(&amp;quot;hello&amp;quot;)"  nohref="nohref" />

It seems the 'save' action of ImageMap url-encodes special characters but the 'open' action doesn't url-decode them.  'open' should convert &quot; back into " so the save action may safely urlencode it again.

I've never done gimp programming before so I doubt I'll be much help in providing a patch.
Comment 1 Michael Schumacher 2013-02-16 15:09:26 UTC
*** Bug 693872 has been marked as a duplicate of this bug. ***
Comment 2 Martin Husemann 2013-02-17 23:00:07 UTC
Created attachment 236503 [details] [review]
Unescape simple xml encodings when reading csim format image maps

This patch seems to fix it for me.

Unfortunately there doesn't seem to be an easy reverse version of g_markup_escape_text() and creating a full GMarkupParser/Context for this looked tremendously overcomplicated.

So a little ad-hoc string magic, but it does seem to do the job.
Comment 3 K Sashi Kumar 2013-02-18 12:22:12 UTC
Created attachment 236583 [details] [review]
Replacement for g_markup_escape_text() to unescape HTML characters

This fixes the problem I guess

Just modified g_markup_escape_text() to recheck the HTML characters every time. g_markup_escape_text() will not unescape the HTML characters.
Comment 4 K Sashi Kumar 2013-03-04 19:12:58 UTC
Created attachment 238032 [details] [review]
Implementation of unmarkup function to unescape the HTML characters
Comment 5 K Sashi Kumar 2013-03-04 19:24:43 UTC
I think this patch would solve the problem. Tried with different test cases and obtained the required result. The patch in comment #2 does not satisfy cases like "sample entity : &sash; ' " . 

The bug is due to the non availability of an unmarkup funtion to unescape the HTML characters. The previous patch (comment #3) is ambiguous. 

The exact problem is: When a map file is saved the special characters are converted to HTML tags. While loading the same map file, the HTML characters are not converted back to the special characters. To avoid this an unmarkup function is needed. 

In this patch I have implemented an unmarkup function to solve the issue.
Comment 6 Mukund Sivaraman 2013-04-06 11:44:28 UTC
Sashi: Several times I had asked you to find out what was causing the patch in comment #2 to not work, instead of writing your own. The patch in comment #2 looked mostly OK to me. If there is something wrong, first explain why it is wrong, not just by providing a testcase that doesn't work, but by explaining *why* "it does not satisfy cases like...". You are a developer. :)

The patch that you attached in comment #4 raises various points:

* It is modifying the generated yacc code, not the .y source file.
* The substitution can be done in-place such as in comment #2.
* The patch in comment #2 uses strstr() to find matching substrings.

The main thing I wanted to get out of this exercise was for you to find what was the fault in an existing patch. :)
Comment 7 Mukund Sivaraman 2013-04-06 11:45:42 UTC
Created attachment 240834 [details] [review]
Fixes patch in comment #2 by using memmove()

strcpy() cannot support overlapping strings.
Comment 8 Mukund Sivaraman 2013-04-06 11:46:51 UTC
Sashi: Please review the patch in comment #7.
Comment 9 K Sashi Kumar 2013-04-08 18:31:49 UTC
Review of attachment 240834 [details] [review]:

Works fine.
Comment 10 Mukund Sivaraman 2013-04-11 08:52:49 UTC
Pushed to master and gimp-2-8 branches:

commit db95e20b0bc09aa81e8c8c706a0a6ca8e4d38824
Author: Mukund Sivaraman <muks@banu.com>
Date:   Thu Apr 11 14:11:00 2013 +0530

    imagemap: Don't use strcpy() in unescape_text() (#649172)
    
    strcpy() doesn't like overlapping strings and this causes other failures
    in this unescaping code.
    
    Also cleanup the code to follow our coding style.

commit bc8a6123c7278a14ea2b6f40b8aa640adb790da0
Author: Martin Husemann <martin@NetBSD.ORG>
Date:   Thu Apr 11 14:09:50 2013 +0530

    imagemap: Unescape simple xml encodings when reading csim format image maps 

Resolving as FIXED, milestone 2.8.

Thank you for the patch Martin, and thank you for the reviews and further attempts Sashi.