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 594435 - Topics in bookmark properties not displayed properly
Topics in bookmark properties not displayed properly
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Bookmarks
2.27.x
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-09-07 22:00 UTC by Emilio Pozuelo Monfort
Modified: 2009-09-08 08:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Display bookmark topics properly (948 bytes, patch)
2009-09-07 22:04 UTC, Emilio Pozuelo Monfort
none Details | Review
also, remove the trailing comma (1.37 KB, patch)
2009-09-08 08:09 UTC, Emilio Pozuelo Monfort
none Details | Review

Description Emilio Pozuelo Monfort 2009-09-07 22:00:56 UTC
Hi,

For any bookmark that has topics, those are not displayed correctly in the bookmark properties. If a bookmark has the "foobar" topic, it will be displayed as "fooba, r", if it as "foo" and "bar", it will be displayed as "fo, bar, o". I.e. the ", " is introduced before the last character for the first topic.

The attached patch fixes the issue for me. Tested with ephy 2.27.91 and gtk+ 2.17.9.
Comment 1 Emilio Pozuelo Monfort 2009-09-07 22:04:18 UTC
Created attachment 142655 [details] [review]
Display bookmark topics properly
Comment 2 Xan Lopez 2009-09-08 07:44:38 UTC
(In reply to comment #1)
> Created an attachment (id=142655) [details]
> Display bookmark topics properly

Looks good!

While you are at it you could make it not add the comma for the last topic too.
Comment 3 Emilio Pozuelo Monfort 2009-09-08 08:09:27 UTC
Created attachment 142677 [details] [review]
also, remove the trailing comma

Sure. This one adds a comma before the topic, rather than after it, but doesn't do it for the first topic (the pos > 0 check). I've tested it with a couple of bookmarks (1 and 3 topics) and works fine.

The other option would be to keep it as is, and after the loop, remove the trailing comma if any, but this looks cleaner to me.
Comment 4 Xan Lopez 2009-09-08 08:11:44 UTC
(In reply to comment #3)
> Created an attachment (id=142677) [details]
> also, remove the trailing comma
> 
> Sure. This one adds a comma before the topic, rather than after it, but doesn't
> do it for the first topic (the pos > 0 check). I've tested it with a couple of
> bookmarks (1 and 3 topics) and works fine.
> 
> The other option would be to keep it as is, and after the loop, remove the
> trailing comma if any, but this looks cleaner to me.

Looks good, please commit.
Comment 5 Emilio Pozuelo Monfort 2009-09-08 08:21:06 UTC
I can't, can you do it for me?
Comment 6 Xan Lopez 2009-09-08 08:34:44 UTC
Committed, will be in 2.28.0.