Item5987: relative TOC anchors get prepended with parameters which may trigger a page reload

pencil
Priority: Normal
Current State: Closed
Released In:
Target Release: minor
Applies To: Engine
Component: TWiki.pm
Branches:
Reported By: TWiki:Main.MarkusUeberall
Waiting For:
Last Change By: OlivierRaginel
Currently, relative TOC links will be created using make_params(0, '#'=>$anchor,@qparams), which effectively _(always) prepends the relative anchor with, e.g., 'style' and 'sortcol' parameters (NB this seems to depend on certain installed plugins, I've never seen this on t.o). However, this behaviour actually can lead to a page reload (tested using Firefox 3.0) if the original topic URL didn't contain parameters (which should be the default case).

As the only purpose for relative TOC links is to scroll down the ready-rendered page, I cannot see why the parameters should be prepended at all (how could they be used?). [cf. clarification below --mue, 15 Sep 2008]

The attached patch patch-item5987-r17489.txt removes the call in question and only uses the relative anchor. [I guess this would break the intended behaviour, so it has to be refined]

(NB _make_params() is also used within lib/TWiki/UI/View.pm to replace %QUERYPARAMSTRING%; basically, all uses should be checked whether they refer to another URI or not.)

-- TWiki:Main/MarkusUeberall - 09 Sep 2008

Update: the behaviour above can also be seen with IE 8.0b2 and Konqueror 3.5.9 (interestingly, the latter only requests the main page for a second time, not the referenced images as well like Firefox/IE)

-- TWiki:Main.MarkusUeberall - 12 Sep 2008

Actually, there is a use for the forementioned parameters: Consider the case where you, e.g., sort a table according to the third column--exactly this configuration will be stored in the TOC links which can then be copied/pasted elsewhere in order to display the topic with the intended sortorder.

However, there is no need to append parameters which don't differ from their default setting (cf., e.g., skin). This means that upon visiting a new topic (w/o some kind of 'persistent' parameters, should they exist, i.e., no parameters), likewise no parameters should show up in the TOC links. Once you, e.g., change a table's sortorder, those links get prepended with associated parameters, but clicking on a TOC item afterwards will not trigger a superfluous reload, because it represents the current representation. Note that the latter case always worked, so atm only the first click for a topic that has initially been loaded w/o parameters causes a reload, since the browser client is unable to determine the current configuration by means of an URI comparison (no parameters vs. specified parameters).

-- TWiki:Main.MarkusUeberall - 15 Sep 2008

I'm having a hard time understanding how we can justify one page reload the first time one clicks on a TOC link. IMHO TOC links must never trigger a page reload because that's what they were designed for. They should be generated by appending the anchor to the URL exactly as it shows in the browser.

-- TWiki:Main.StephaneLenclud - 16 Oct 2008

Actually a cheap way to get the correct behavior without reload is to use:

%ENV{"REQUEST_URI"}%#Anchor

/Tasks/Item5987#Anchor

Anchor

-- TWiki:Main.StephaneLenclud - 16 Oct 2008

With TWikiStandAlone in mind I think you cannot make sure, REQUEST_URI always exists.

-- TWiki:Main.OliverKrueger - 16 Oct 2008

Yeah wink I said it's a cheap way but surely we should be able to get REQUEST_URI through some TWiki APIs or add a new API if needed to provide developer with the equivalent from REQUEST_URI regardless of the environment you are running.

-- TWiki:Main.StephaneLenclud - 16 Oct 2008

Well, I think, stripping all (irrelevant) parameters is not an option. There might be some cases, in which TWiki can simplify a complex URI to just "#anchor". But I consider it as a browsers bug / job to handle equal URIs correctly.

Suggest to close this Item with no action.

-- TWiki:Main.OliverKrueger - 22 Oct 2008

I have to disagree with that. IMHO TOC links must not trigger page reload. It's not about stripping here it's about not adding parameters. On my TWiki running NatSkin it adds the skin parameter to the TOC links and that's wrong because it triggers page reload for no reason.

-- TWiki:Main.StephaneLenclud - 24 Oct 2008

As shown here, the optional query part makes it impossible for the client (browser) to guess whether a page has to be reloaded or not, so it's definitely not a browser bug (that's what I wanted to state above: if the query part would always be appended by TWiki, the problem (also) wouldn't exist, but this may look "ugly").

-- TWiki:Main.MarkusUeberall - 24 Oct 2008

Definitely not a browser bug indeed.

I patched my installation by using "$ENV{'REQUEST_URI'}#$anchor" instead of _make_params(0, '#'=>$anchor,@qparams) in TWiki.pm around line 2034. It does not trigger page reload since it's not appending skin parameter anymore. It works just fine in respect to table sorting and any URL parameters. Obviously it won't work with TWiki standalone. If there is a TWiki API layer that would return REQUEST_URI in standalone and CGI mode then we should use that API instead of $ENV{'REQUEST_URI'}.

-- TWiki:Main.StephaneLenclud - 26 Oct 2008

This seems to be fixed.

From the code of _make_params, it checks when it's an anchor, and treats it as an anchor, and puts it at the end.

Thus, closing this bug. Re-open it if you can re-produce it. I tried with DocumentGraphics (DocumentGraphics on Foswiki 1.0), and it works.

It also solves Tasks.Item5928

-- OlivierRaginel - 17 Dec 2008

ItemTemplate edit

Summary relative TOC anchors get prepended with parameters which may trigger a page reload
ReportedBy TWiki:Main.MarkusUeberall
Codebase
SVN Range TWiki-5.0.0, Mon, 18 Aug 2008, build 17431
AppliesTo Engine
Component TWiki.pm
Priority Normal
CurrentState Closed
WaitingFor
Checkins
TargetRelease minor
ReleasedIn
I Attachment Action Size Date Who Comment
patch-item5987-r17489.txttxt patch-item5987-r17489.txt manage 985 bytes 09 Sep 2008 - 17:23 MarkusUeberall Proposed patch
Topic revision: r14 - 17 Dec 2008, OlivierRaginel
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy