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 714314 - Use system-theme icon as generic profile picture
Use system-theme icon as generic profile picture
Status: RESOLVED FIXED
Product: geary
Classification: Other
Component: client
unspecified
Other All
: Normal normal
: 0.12.0
Assigned To: Geary Maintainers
Geary Maintainers
Depends on: 765516
Blocks:
 
 
Reported: 2012-08-31 11:27 UTC by Geary Maintainers
Modified: 2016-10-07 13:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fallbackicons.png (77.23 KB, image/png)
2013-06-23 09:54 UTC, Geary Maintainers
Details
diff.txt (3.81 KB, text/plain)
2013-06-23 09:54 UTC, Geary Maintainers
Details

Description Charles Lindsay 2013-11-21 20:26:02 UTC


---- Reported by geary-maint@gnome.bugs 2012-08-31 04:27:00 -0700 ----

Original Redmine bug id: 5747
Original URL: http://redmine.yorba.org/issues/5747
Searchable id: yorba-bug-5747
Original author: Alexander Wilms
Original description:

It'd add some desktop consistency if you used the icon that's also visible in
the session indicator.



---- Additional Comments From geary-maint@gnome.bugs 2013-11-14 17:15:00 -0800 ----

### History

####

#1

Updated by Jim Nelson about 1 year ago

Could you clarify this? Where should the session icon be used inside of Geary?
Do you mean as the avatar displayed in the conversation view?

####

#2

Updated by Alexander Wilms about 1 year ago

Yes, exactly.

####

#3

Updated by Alexander Wilms 10 months ago

Could you give me a hint where the icon is set? I also couldn't find any file
for the generic picture.

####

#4

Updated by Eric Gregory 10 months ago

It comes from Gravatar. See util-gravatar.vala and the Gravatar API reference:
https://en.gravatar.com/site/implement/images/

The thing to do here would be to have Gravatar return a 404 instead of one of
their generic images.

####

#5

Updated by Jim Nelson 10 months ago

  * **Category** set to _client_

This might be a little bit of a trick to implement. We don't load the Gravatar
image then copy it into the ConversationViewer. Rather, since the
ConversationViewer is a WebKit control, we simply inject the Gravatar URL into
the HTML and let WebKit handle the rest. If we changed the Gravatar URL to
issue a 404 when no Gravatar is found, then we'd simply be injecting broken
images into the ConversationViewer.

I'm not sure how to get around this, but since this is not mission-critical
functionality, the simpler, the better. I would be cautious of any solution
that involved lots of fixups and error checking throughout the code.

####

#6

Updated by Alexander Wilms 10 months ago

Wouldn't the 'try' fail if it returns a 404? I thought that one might be able
to set the fallback icon in the catch block.

####

#7

Updated by Jim Nelson 10 months ago

That warning message is not properly worded. Gravatar.get_image_uri() doesn't
throw an Error, it merely generates a string. I think that Error is being
thrown by set_attribute(), which has nothing to do with loading the Gravatar.

I'll go ahead and correct this warning message. But the broader issue remains,
which is trapping the 404 and replace it with our own icon.

####

#8

Updated by Alexander Wilms 5 months ago

Any idea why this doesn't work? Maybe a policy restricting from where stuff
can be loaded?

    
    Geary.RFC822.MailboxAddress? primary = message.sender;  
            if (primary != null) {  
                try {  
                    WebKit.DOM.HTMLImageElement icon = Util.DOM.select(div_message, ".avatar")  
                        as WebKit.DOM.HTMLImageElement;  
                    icon.set_attribute("src",  
                        Gravatar.get_image_uri(primary, Gravatar.Default.NOT_FOUND, 48));  
                } catch (Error error) {  
                    try {  
                        WebKit.DOM.HTMLImageElement icon = Util.DOM.select(div_message, ".avatar")  
                            as WebKit.DOM.HTMLImageElement;  
                        icon.set_attribute("src", "/usr/share/icons/Humanity/actions/48/help-contents.svg");  
                        debug("Failed to inject avatar URL: %s", error.message);  
                    } catch (Error error) {  
                        debug("Failed to load themed icon: %s", error.message);  
                    }  
                }  
            }

####

#9

Updated by Alexander Wilms 5 months ago

That should read file:///usr/share...

####

#10

Updated by Jim Nelson 5 months ago

It may be that WebKit doesn't support SVG. We certainly don't want to place
hard-coded paths in the code.

####

#11

Updated by Robert Schroll 5 months ago

I think it's a policy restriction. WebKit doesn't allow for the loading of
resources from file:// unless the HTML page is from file://. This doesn't
happen with WebView.load_string unless the fourth argument is a file:// URL.

While we could do that, I think a better approach would be to convert the
desired file to a data URL. This is what we do for the various icons we
already use. ConversationWebView.set_icon_src() is the function to look at.

####

#12

Updated by Alexander Wilms 5 months ago

I wanted to do this step by step, that's why I used a hardcoded path.

I'll try set_icon_src()

####

#13

Updated by Alexander Wilms 5 months ago

I made some changes, but it still doesn't work.

I made set_icon_src() public,

changed the HTML template a bit: <div class="gravatar"><img src=""
class="avatar" /></div>

and tried to set the icon in ConversationViewer:

    
    Geary.RFC822.MailboxAddress? primary = message.sender;  
            if (primary != null) {  
                try {  
                    WebKit.DOM.HTMLImageElement icon = Util.DOM.select(div_message, ".avatar")  
                        as WebKit.DOM.HTMLImageElement;  
                    icon.set_attribute("src",  
                        Gravatar.get_image_uri(primary, Gravatar.Default.NOT_FOUND, 48));  
                } catch (Error error) {  
                    web_view.set_icon_src("#email_template .gravatar .avatar", "help-contents");  
                }  
            }

####

#14

Updated by Robert Schroll 5 months ago

Well, the CSS rule "#email_template .gravatar .avatar" isn't going to match
anything, since I don't think we have a ".gravatar" class anywhere.
Futhermore, this code is probably running after we clone the #email_template
to make the specific div for the message, so we don't want to adjust the
avatar in the template anymore.

However, this does suggest a better course of action -- set the src of the
".avatar" image in the template before it's cloned. Then if we don't get an
image from Gravatar, we're already set up to use the system one. This could go
in the same place as all the other calls to set_icon_src.

####

#15

Updated by Alexander Wilms 5 months ago

I added a .gravatar div as mentioned above. Would removing #email_template
from the selector be enough to make it work in theory?

Good idea. Do you know of a way to determine if a link returns a 404?

Maybe try to create a new file from the url in the try {} block? If it's a
404, it fails and the original src remains. Right now it just shows a 'missing
picture' icon

####

#16

Updated by Alexander Wilms 5 months ago

Using LibSoup's KnownStatusCodes one can determine if an avatar is available.
But even if I comment out the part where the icon is now condictionally set,
the icon set in the original html template doesn't appear in the web view when
using Geary. Could anyone shed a light on this?

    
    if (primary != null) {  
                try {  
                    string gravatar_uri = Gravatar.get_image_uri(primary, Gravatar.Default.NOT_FOUND, 48);  
                    var session = new Soup.SessionAsync ();  
                    var avatarmessage = new Soup.Message ("GET", gravatar_uri);  
                    Soup.KnownStatusCode avatarstatus = (Soup.KnownStatusCode) session.send_message ( avatarmessage);  
                    print(avatarstatus.to_string() + "\n");  
                    if(avatarstatus == Soup.KnownStatusCode.OK) {  
                        WebKit.DOM.HTMLImageElement icon = Util.DOM.select(div_message, ".avatar")  
                            as WebKit.DOM.HTMLImageElement;  
                        icon.set_attribute("src", gravatar_uri);  
                    }  
                } catch (Error error) {  
                    debug("Failed to inject avatar URL: %s", error.message);  
                }  
            }

####

#17

Updated by Alexander Wilms 5 months ago

  * **File** Fallbackicons.png added
  * **File** diff.txt added

Now it works, but it's a bit slow. Should probably be loaded async

####

#18

Updated by Jim Nelson 5 months ago

  * **Status** changed from _Open_ to _Review_
  * **Assignee** set to _Alexander Wilms_
  * **Target version** set to _0.4.0_

Ok, we'll take a look at it. I'm pretty sure we're going to need this to be
done using a Soup async call, not just because it's slow, but also so it
doesn't hold up the client (cause a brief hang) while it queries for the
avatar.

####

#19

Updated by Robert Schroll 5 months ago

Another approach would be to send to requests off to gravatar and then deal
with the fallout when it can't find an image. The WebView has two signals that
could be useful. "resource-load-failed" would seem to be what we want, but in
testing I don't see this firing. Maybe by failure they mean something more
drastic than a 404. The other is "resource-response-received", which fires
whenever we get a response, good or bad. We'd have to look through these for
404 responses to gravatar requests to find the URLs requested, and then
replace those in the document with the data URLs of the system avatar. This
might be a bit more work, but it's probably less complicated than setting up a
async call, and it avoids making two requests for the same resource.

####

#20

Updated by Jim Nelson 4 months ago

  * **Status** changed from _Review_ to _Open_

I took a look at this and think Robert's approach is closer to what we need to
do here. The basic problem is speed -- there's just too much jumpiness as the
conversation loads looking for gravatars. I don't even think using an async
Soup call will fix the problem. But Robert's approach (letting WebKit drive
discovering the 404s and then fixing up the DOM nodes to use the system icon)
makes more sense.

####

#21

Updated by Jim Nelson 3 months ago

  * **Target version** changed from _0.4.0_ to _0.5.0_

####

#22

Updated by Jim Nelson 7 days ago

  * **Target version** deleted (<strike>_0.5.0_</strike>)



--- Bug imported by chaz@yorba.org 2013-11-21 20:26 UTC  ---

This bug was previously known as _bug_ 5747 at http://redmine.yorba.org/show_bug.cgi?id=5747
Imported an attachment (id=260943)
Imported an attachment (id=260944)

Unknown version " in product geary. 
   Setting version to "!unspecified".
Unknown milestone "unknown in product geary. 
   Setting to default milestone for this product, "---".
Setting qa contact to the default for this product.
   This bug either had no qa contact or an invalid one.
Resolution set on an open status.
   Dropping resolution 

Comment 1 Michael Gratton 2016-04-26 13:25:09 UTC
Bug 765516 is using avatar-default-symbolic for the generic avatar image.
Comment 2 Michael Gratton 2016-10-07 13:35:27 UTC
Fixed on master by 898fa33, as part of Bug 728002.