Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support relative links in the plugin output #2916

Closed
Thomas-Gelf opened this issue Aug 10, 2017 · 12 comments

Comments

@Thomas-Gelf
Copy link
Contributor

commented Aug 10, 2017

Writing a Plugin I cannot make an assumption about your Icinga Web 2 baseUrl. How do I create links pointing to somewhere in the web or to some of it's modules in my plugin output?

@lippserd

This comment has been minimized.

Copy link
Member

commented Aug 10, 2017

I don't know man. Please ask your question on the forum or on our mailing list :D

@Thomas-Gelf

This comment has been minimized.

Copy link
Contributor Author

commented Aug 10, 2017

😆

I guess there is no way, so please interpret my question as a feature request ;)

@lippserd lippserd changed the title Plugin output - relative links Support relative links in the plugin output Sep 21, 2017

@lippserd

This comment has been minimized.

Copy link
Member

commented May 6, 2019

Hi Tom,

Could you provide a possible use case here? Custom host/service actions sound sufficient to me here.

Best,
Eric

@Thomas-Gelf

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

@lippserd: separation of concerns. A plugin cannot and should not know where your Web 2 installation is located. Sometimes it's completely impossible, as multiple web front-ends might serve the same IDO via multiple domains/URLs. Custom actions: they are not related to this issue, links in the Plugin Output should "just work".

@lippserd

This comment has been minimized.

Copy link
Member

commented May 6, 2019

I don't see why custom navigation items are not related to this issue. What is real use case for this? Which checks do you have in mind here?

@Thomas-Gelf

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

Ever plugin writing something like this to it's stdout:

Everything is fine, click <a href="module/controller/action">here<a> for details.

We use this logic everywhere, and we allow links in plugin outputs. I see a perfect fit for this.

@lippserd

This comment has been minimized.

Copy link
Member

commented May 6, 2019

Well, ok. Why not 😆

@lippserd lippserd removed the needs-feedback label May 6, 2019

@lippserd lippserd added this to the 2.7.0 milestone May 6, 2019

@nilmerg nilmerg self-assigned this Jun 4, 2019

@nilmerg

This comment has been minimized.

Copy link
Member

commented Jun 5, 2019

Hey Tom,

We use this logic everywhere, and we allow links in plugin outputs. I see a perfect fit for this.

Where exactly? If you refer to e.g. $this->href('monitoring/list/hosts') this is not a relative url at the end. Cases where real relative links are in use (to my knowledge) are located in our documentation only.

I don't ask because I don't like the idea. I ask because I really need proper examples (the documentation doesn't count) as I see two possible solutions here:

a) A proper base path (e.g. <base href="/icingaweb2/">) and an adjusted loader.js which will apply to everything globally
b) A transformation specific to relative links in pluginoutput

a) will have a reasonable impact on all relative links currently in use. Previously they were rooted at the current browser location. Once a base path is defined this is not the case anymore. So this will possibly be a breaking change since it also applies to named anchors. (#foo)
b) on the other hand is only applied to pluginoutput (or generally speaking, external html input) which could have never successfully utilized relative links previously. So this wouldn't be a breaking change at all.

I like a) more because it's more flexible and future proof. Though, if we don't want such a change or have massive amounts of relative links out in the wild (which I highly doubt) relying on the current behavior I'll stick to b).

nilmerg added a commit that referenced this issue Jun 6, 2019

nilmerg added a commit that referenced this issue Jun 6, 2019

@lippserd

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

I opt for setting the base href. Is the loader change necessary? Why did you change this?

@nilmerg

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

The base href is only respected by the browser, not by XHR.

Browser behavior:
foobar + base href
#foobar + base href
/foobar is used as is

Note that anchor. Any such relative links will not work anymore with #3819. Our JS of course can still process such without the base href. Though, the browser will not.

@lippserd

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

I'm quite sure that XHR respects the base URL. Do you have any proof that this is not the case? You mean that the anchor links won't work without JS right? Because our JS should have no problems to deal with this.

@nilmerg

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

During my tests XHR showed no interest at all for it. And that's still the case. With the anchor I mean that <a href="#foo">To Section Foo</a> will result in /icingaweb2/#foo with a base href.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.