Hires links on images work when set from Kupu, not from Forms editor

[Guido Wesdorp]

When creating an image with a link to its hires version in Kupu, it works like a
charm. When doing the same with the Forms editor, the link points to the scaled
down version, not to the hires one. It looks like Kupu creates both an attribute
called 'link_to_hires' with a value of '1', and appends '?hires' to the URL in
the link field, while the Forms editor only creates the attribute, not the URL
prefix.

I'm not sure which of those two behaviours is right, it sounds to me like having
both an attribute and a prefix is redundant, but obviously some choice should be
made and applied consistently in order for the forms editor and Kupu to both
work properly.

[Martijn Faassen (faassen)]

The design with the attribute in the XML indicating you want a highres link sounds
better than the extra parameter in the URL -- the renderer could and should take
care
of this. What does Kupu do when you switch back from highres? Rip off the ?hires
bit too?

We need to verify what both renderers (both xmlwidgets as well as XSLT) do when
generating hires links.

If we drop the requirement for ?hires we don't expect backwards compatibility
issues. We do need to check what happens with links that *already* have this
?hires bit in them.

[Martijn Faassen (faassen)]

This one is too hairy to tackle before the beta, so deferring it.

[Martijn Faassen (faassen)]

Deferring this one for Silva 1.5.

[Martijn Faassen (faassen)]

Will look into this in SIlva 1.6 era.

[Andy Altepeter (aaltepet)]

Note: consistency issues aside, this is not a problem with the output of the xslt renderer, only the widgets-based renderer.

[Andy Altepeter (aaltepet)]

I've adjusted the widgets renderer to append "hires" when link_to_hires="1" and "?hires" isn't on the link. The xslt renderer has the correct link in "rewritten_link" from the document xml export...the logic is already done there.

The two renders actually take different approaches to rendering the link.
Widgets: If "link" attribute, use it. Add '?hires' if necessary
XSLT (in xmlexport, prior to transform): transform "path" attribute to a url. If "link_to_hires"==1, then append "?hires" onto the url-path and use that as the link. XSLT ignores the "link" attribute altogethor if the link is to ?hires.

The "?hires" 'feature' in kupu was prob. added when kupu was being implemented, and this widgets bug was noticed. Note the bug was never fixed. Then the xslt renderer came along, and took a different approach. The two have never been reconciled.

I recommend at minimum that kupu no longer appends '?hires' to links.

The question is:
1) when should this happen
and
2) should there be an upgrade script to remove '?hires' from the "link" attributes?

Note that the reason for this issue (links not working in forms editor) is resolved, but now we need to consider damage control. If there will be an upgrade script, damage control needs to be moved to 2.1 milestone (and a separate issue created for that). But, should kupu be fixed for 2.0.1, or 2.1?

[Andy Altepeter (aaltepet)]

Briefly looked into possibly adjusting kupu, but it happens in the magic SilvaDocument/transform code. Not knowing this area of code, I feel adjusting it to remove the "?hires" upon saves could be a major change.

[Kit Blake (kitblake)]

I just changed this to 2.1. We need to do a 2.0.1 release soon, and as you suggested, any damage control should be in 2.1, so let's leave 2.0.1 in as stable a state as possible.

[raoul.schaffner (raoul-schaffner)]

observed behaviour in silva 2.0.1 (without silvalayout) and kupu 1.4b1:

a) set "link to high resolution version", save&publish
--> the link will be "<image-url>?hires?hires"

b)
1.set image's link to "<image-name>?hires", don't set the hires flag, save&publish
--> the link will be "<image-url>?hires"
2. reopen the document
--> the link-field will be empty, the hires flag set
3. save&publish
--> the link will be "<image-url>?hires?hires"

[Eric Casteleijn (thisfred)]

Andy Could you have a brief look at this new report and see if it can be solved quickly?

[Andy Altepeter (aaltepet)]

Bug report from raoul.schaffner has been fixed and checked into both the SilvaDocument trunk and the SD 2.0 branch.

[Andy Altepeter (aaltepet)]

Note: follow up on changing kupu so that it no longer appends "?hires" to the link_to url in the silvaxml image tag. Create an upgrade script to bring both editors inline.

[Kit Blake (kitblake)]

Is this upgrade script for correcting inconsistent arguments in existing content? Such as [image-url]?hires?hires''?
I ask because upgrade scripts are painful on large sites. I wonder if we could make it optional.

[Andy Altepeter (aaltepet)]

Yes, this upgrade script would be for correcting inconsistencies in existing content. I hadn't considered fixing the case if '?hires?hires'. My thought was that currently in the xml for linking to hires, you have one of two possible formats:

<image link_to_hires="1" url="image-url" />

OR

<image url="image-url?hires" />

OR actually a combination of the two.

This comment https://bugs.launchpad.net/silva/+bug/101567/comments/6 more fully explains the issue and the possible need for reconciling. It would be nice to standardize on one method (link_to_hires, or ?hires). Especially since we want to add support for linking to thumbnails as well. It actually would be better / more scalable to have a link_to_image_format="hires|thumbnail|web" and forgo this "?hires" thing altogether.

[Kit Blake (kitblake)]

Quoting from your https://bugs.edge.launchpad.net/silva/+bug/101567/comments/6

> I recommend at minimum that kupu no longer appends '?hires' to links.

Sounds fine.

> The question is:
> 1) when should this happen

This is something we'll change in 2.1.

> 2) should there be an upgrade script to remove '?hires' from the "link" attributes?

Do they break anything? Also, if and when a document with image gets edited, it'll be fixed, yes?. Best would be that there's an upgrade script available for any site manager that needs it, but it's not part of the upgrade (i.e. 'behind' the upgrade button in service_extensions).

[Andy Altepeter (aaltepet)]

OK, kupu will now no longer append '?hires' to these urls. This will happen in 2.1.

> Do they break anything?

No, nothing is broken. And, good point, they will be fixed when a document is edited. In light of that, I don't think an upgrade script is really worth the effort.

So, marking this issue as "fix committed"