Code review comment for lp:~deryck/launchpad/hot-bugs-list-color-icons-439128

Revision history for this message
Brad Crittenden (bac) wrote :

Hi Deryck,

Thanks for the branch. It all looks good...except it's a huge regression for webkit-based browsers. We go from always having a grey icon to having NO icon at all.

As Curtis immediately saw, your <span> is wrong in that it is invalid to do a self-closing <span />. So your changes like:

<span tal:attributes="class context/default_bugtask/image:sprite_css" />

need to be:

<span tal:attributes="class context/default_bugtask/image:sprite_css"></span>

Please be a superstar and find the other places in the codebase where this happens. Your demo URL https://bugs.edge.launchpad.net/launchpad/+bugs is currently broken too.

Looking at your new test code I'm a bit confused by:
+ >>> print high_bugs[0]
61 + <span class="sprite bug-high">
62 + </span>

Because when I look at the html for bugs.launchpad.dev I see self-closing spans. Can you figure out why that is happening?

review: Needs Fixing (code)

« Back to merge proposal