Edgewall Software

Ticket #6562 (closed enhancement: fixed)

Opened 13 months ago

Last modified 12 months ago

[patch] Absolute reference to logo for rss and notification

Reported by: osimons Owned by: osimons
Priority: normal Milestone: 0.11
Component: rendering Version: 0.11b1
Severity: normal Keywords: logo chrome
Cc:

Description

The current chrome.logo implementation is not really complete with regards to references to the logo, and for a normal use case (like the default) it is very cumbersome to use as it returns it as server-root absolute (like /myproject/chrome/site/mylogo.png) instead of fully absolute with scheme and servername, or strictly project-relative like most other resources. That means you can't just call abs_href.chrome() on the logo reference either when you need that for rss or html email.

Also, we return to the template a variable called chrome.logo.src_abs (as True or False) that is only used one place - in the timeline.rss template. Additionally, it does not work there and no logo is displayed for timeline feeds. Here is that faulty code and the use for src_abs that makes no sense as a boolean:

<url>${chrome.logo.src_abs and chrome.logo.src_abs or abs_href(chrome.logo.src)}</url>

I've made a patch for this behavior whereby we change the chrome.logo.src_abs to actually return a reference to the absolute location - as good as can be calculated at least. Current behavior for chrome.logo.src as a server relative path has not been changed, so this should not impact current uses of that (see tests for confirmation).

Summary:

  • get_logo_data() takes both a href and optionally an abs_href and returns both the relative (src) and absolute path (src_abs) always - as correct as possible based on the data we have for each use case at least.
  • Updated the various feed templates to use the absolute reference directly.
  • Added some tests for this additional behaviour.

An open TODO is left for possibly using chrome.htdocs_location for logos in common htdocs, but with the option to set a full reference anyway this should not be a big deal. Further changes are needed to get that variable in and used. Is it needed, or just let it be?

Attaching the patch to this ticket for review / approval before committing - and leaving some docs for the future :-)

Attachments

t6562-r6370-logo_abssrc-a.diff (7.2 KB) - added by osimons 13 months ago.
Initial patch for returning both a relative and absolute path to the logo.

Change History

Changed 13 months ago by osimons

Initial patch for returning both a relative and absolute path to the logo.

Changed 12 months ago by osimons

  • milestone changed from 0.11.1 to 0.11

Chatted with eblot, and we decided to bump the priority on this - it should be in 0.11 as it is among other things needed to make our RSS valid for most common logo setups. See #6556 for related discussion.

Changed 12 months ago by osimons

  • keywords review removed
  • status changed from new to closed
  • resolution set to fixed

Fix based on patch applied in [6384].

Add/Change #6562 ([patch] Absolute reference to logo for rss and notification)

Author



Change Properties
<Author field>
Action
as closed
Next status will be 'reopened'
to The owner will change from osimons. Next status will be 'closed'
 
Note: See TracTickets for help on using tickets.