ubc / wiki-embed

WordPress plugin that lets you embed mediawiki pages into your site, sites like Wikipedia
19 stars 10 forks source link

Typo in WikiEmbed.php #7

Open JulianSMoore opened 9 years ago

JulianSMoore commented 9 years ago

Hi Enej

1st - great plugin! If you could only find a way to bring it up to WP4 I would be so grateful.

I have started to learn JS & PHP to be able to understand how it all works and have found 1 issue by code inspection and 1-2 by behaviour. I did an accordion.js fix myself to make them fit the text, but I can't resolve the following...

WikieEmped.php at line 153 current_user_can( 'pulish_pages' ) should be current_user_can( 'publish_pages' )

Note however that making the correction uncovers a problem (further incompatibility with WP4.0?)

With settings as Internal WIki Links = New Wordpress page, and wiki embedded with accordions

As coded, clicking on a wiki link creates a new page with nominally correct accordions; correcting typo results leads to full width, all expanded, uncollapsible accordions.

Note also (independent of this typo) the message:

Warning: in_array() expects parameter 2 to be array, null given in /var/sites/b/blog.pantologea.com/public_html/wp-includes/nav-menu-template.php on line 609

Which I assume may be a WP4 incompatibility issue. Installing 1.4.7 from here also led to "can't wiki embed content from your own site" is that a bug or a (retrograde) new feature.

It's so close to being perfect for my needs...

wiki-embed misbehaviour

loongchan commented 9 years ago

Hi JulianSMoore, I just committed some code which should fix at least some of the issues. If you could re-download and confirm that the issues were addressed, it would be greately appreciated.

Best, Loong

JulianSMoore commented 9 years ago

Hi Loong

Wow - I wasn' texpecting to see such a rapid response. Thanks. I will download and retry.

Extra info since yesterday: I have found that one problem is a theme compatibilty issue: things weren't so bad using WP2014 instead of my ElegantThemes Divi. Have raised the issue with them and will coordinate with findings.

Cheers,

Julian

JulianSMoore commented 9 years ago

Hi Loong,

I can't embed my own blog anymore; "Sorry can't wiki embed content from your own site" - is that fixable? It is listed with wikipedia in settings security "Restrict the urls of wikis that you want content to be embedded from. This way only url from line incomplete ) It was OK in 1.4.6.

I referenced Wikipedia instead to test.

Just downloaded and tested. Same problem with Divi (their problem) but with WP2014 it's OK except that the accordions on New Pages from clicked wiki links are missing header boxes and icons etc. - and big graphics is overflowing (I'm less concerned with that, but FYI)

I also tried out the Overlay which didn't work well before with Divi - don't think I tried it with WP2014.

Note that the extra styling for accordions & tabs was enabled.

Results attached.

Cheers,

Julian

saturn article in wp2014 with wiki-embed 1 4 7 from github 2014-10-23

link from saturn article in wp2014 with wiki-embed 1 4 7 from github 2014-10-23

link as overlay from saturn article in wp2014 with wiki-embed 1 4 7 from github 2014-10-23

loongchan commented 9 years ago

Hi Julian, The ability to include your embed content from your own site was removed due to possible performance issue (basically it can include itself in special cases, where it would eat up all the server memory and die). So it's "feature".

I fixe a bunch more warnings that was triggered on certain cases.

Thanks for the help testing. Please try it again. I'm hoping that fixing up the warnings will help with the compatibility with other themes.

Best! Loong

JulianSMoore commented 9 years ago

Hi Loong,

Thanks again for the quick response. I'm glad I can help and will be very happy to conduct more testing.

Report on latest changes; settings generally as shown in image 3 below.

1/ Effect on WP2014 theme

1.1/ Overlay continues to present two vertical scrollbars - note that the double scroll bar does not appear when using Divi (see image 2 below)

1.2/ New Page continues to lack accordion headers, icons, etc. and (confirming) all accordions are opened.

2/ With Divi theme

2.1/ nav-menu-template warning has gone. Great! [For my education, I will look at the diffs to see what change(s) fixed that - though if you can point me in the right direciton that would save time.]

2.1/ New observation per image 1 below: overlay close button wasn't initially visible ?because overlay rect seems to have blank space at the bottom. Suggestions: fix overlay height and/or give overlay a strong border to make its extent apparent; also "close"/"X" at top would ensure overlay can be dismissed without scrolling down (actually I think the overlay sizes to the window and maybe

2.2/ New observation per image 2 below: whilst placement of overlay is fine in WP2014 (IIRC) the increased menu height in Divi is not accommodated - top of overlay is hidden.

Re: no loading own site

Can you say how the self-include could happen? I am prepared to accept general performance limitations of two things running on the same server and if I can avoid creating the circumstances leading to infinite regress I'll do so.

If the conditions can be plainly stated I would suggest that it should be a user choice - perhaps with own-site embed disabled by default with a warning & info beside the option to enable. IIRC the feature is a simple conditional and I will try commenting it out later.

Cheers,

Julian

aphelion link as overlay from saturn article in divi theme with wiki-embed 1 4 7 of 2014-10-24

sun link as overlay from saturn article in divi theme with wiki-embed 1 4 7 of 2014-10-24

wiki-embed 1 4 7 of 2014-10-24 settings for testing

JulianSMoore commented 9 years ago

Correction!

Sorry Loong, not enough caffeine in the system yet ;) The nav-menu-template issue is still present on open as new-page in the Divi theme as shown below :(

Whether the accordion size etc. issues are also their problem I don't know!

Ah! It's been a long time since I was involved in testing with two independent developers...

I nearly closed the issue at ElegantThemes but have corrected the update there as well.

Cheers

Julian

sickle link as new page from saturn article in divi theme with wiki-embed 1 4 7 of 2014-10-24

JulianSMoore commented 9 years ago

PS Please let me know whether you would like me to redocument as a new issue so that you could close the "typo" issue down.

JulianSMoore commented 9 years ago

Hi Loong,

Some additional observations. (with disallow own-site disabled by commenting out IF @ line 265, WP2014 theme unless otherwise stated.)

1/ General: options syntax - "accordion" is singular, "tabs" is plural; probably too late to make it "accordions" and "tabs".

2/ Wiki Content box links not not working, i.e. with wiki Content box also shown

2.1/ With tabs, clicking on a link in the Content box only goes to if the tab is active; ideally it should activate the appropriate tab for the heading or sub-heaing. If this is not achievable Content should be disabled if either tabs or accordions are used.

2.2/ With accordions, nothing happens at all; ideally should open the correct accordion etc.

3/ With mediawiki extension CIte, and accordions, clicking a citation ref does nothing because, like 2.1 above, the accordion is closed. If the references accordion is opened obviously there no clickable citations in my doc (wiki text before 1st heading should always be displayed so could always be found I suppose). Similarly, clicking the citation return in the citation accordion does nothing - but for a different reason: "This url does not meet the site security guidelines". I haven't checked, but I would expect the same for tabs.

NB Links don't work - blog not open to public for obvious reasons!

Good link example: http://blog.pantologea.com/?wikiembed-url=http%3A%2F%2Fblog.pantologea.com%2FKumu%2FOutcome&wikiembed-title=outcome

bad citation link example: http://blog.pantologea.com/uncategorized/wiki-embedding-test-post-with-citations/?refresh=e6905be17d#cite_note-1

4/ One other thing: with wiki-links: if the wiki target does not exist, clicking on a "placeholder" such as would appear in wiki with a red link loads the wiki create page! Sample: "http://blog.pantologea.com/Kumu/index.php?title=Service&action=edit&redlink=1" If it were possible to trap the "redlink" it would be nice to avoid invoking the wiki directly, especialy since Wiki-Embed otherwise completely screens it.

Cheers,

Julian

loongchan commented 9 years ago

Hi Julian, 1/ The reason from what I understand is that there can be more than one tab, thus tabs. Accordion, isn't that a grouping thing? The accordion has multiple sections, but each section is not called an accordion, for accordion, shouldn't the aggregate accordion sections be called accordion?

2/ I tried the shortcode with content box and it seems to link out properly. If you could also include the shortcode being used whenever you have examples, it would greatly help me diagnose the issue (if no tabs or accordions. I'll have a look at this as part of 2.1).

2.1/ Could you give a concrete example for me to look at? specific wiki page included, specific short code options used, which link on which tab. I tried [wiki-embed url='http://en.wikipedia.org/wiki/Saturn' tabs no-edit no-infobox ]and I could still click on tabs. If you are referring to using tabs and you click on a citation link for example, you expect it to go open the tab where the link is, then scroll down to the section where the link is set for right? If that's the case, It will require some custom javascript work to get working. I'll chat with the folks here to see if it's a well requested feature.

2.2/ If I fix it for tabs, it should be similar to fixing it for accordions

3/ I'm hoping that if I do 2.1, it should fix 2.2 and 3

4/ Sorry, I don't quite get what is requested here.

JulianSMoore commented 9 years ago

Hi Loong

Quick response before I go to develop and document a concrete example for you to look at - if you could give me an email address I could temporarily set you up as an administrator (so you can toggle themes). Don't know how to PM you here - what's the best communicate while retaining privacy?

One other piece of news: because some things work in WP2014 but not in Divi, I raised the issue with ElegantThemes; they were reluctant at first ("can't support all 3rd party plugins") but responded positively to a persuasive argument and are also looking into the in-array Warning (nav-menu-template) problem. I'll pass on anything they tell me as soon as I hear from them.

1/ Fair point; an accordion is an instrument whose body is collapsible, but the sections aren't called accordions. On the other hand, each section with a header and control has a part that can expand/collapse so I thought of each section as being the accordion. However (just to emphasise I'm agreeing with you) the sections work together - as once opens another closes, (Had it been possible to have more than one section expanded at a time I might have thought of al the sections as comprising "the" accordion from the beginning.)

2, 3/ TBD

4/ In mediawiki, links to other pages are marked up as [[page_title]] (double square brackets)

If the target page_title exists, mediawiki makes it a hyperlink to page_title and styles it blue to indicate that. Clicking the link then calls up the target page.

If page_title does not exist a different hyperlink is created to take the user to page creation page for "page_title" and the link is styled red (hover tooltip "... does not exist"). The url string that does this is "index.php?title=Service&action=edit&redlink=1" the hyperlink takes the user.

At present, clicking on such "red" links in a wiki-embedded page takes the user directly to the page creation page of the wiki itself, whereas all other (blue) links are intercepted and generate a new WP page. Ideally, clicking on a red link should do nothing.

Now, it happens that I have locked down the wiki so that only my IP can access it directly, so whilst other blog users would not (in my case) get access to the wiki, they would at least get an error (404?), and it would be nicer if red links did nothing.

Best regards

Julian

JulianSMoore commented 9 years ago

Update: clarification - content box works (mostly OK) on embed without Tabs or Accordion. Item 2 was ambiguous - it was meant as a header for 2.1, 2.2 not to imply a problem with Content box generally.

One exception noted: with the contents as in the illustration below, clicking on headings in the content box scrolls the window to the relevant heading... except on heading 5 which causes a scroll that leaves heading 6 at the top of the page.

More info as I develop it.

capability contents box

loongchan commented 9 years ago

Hi Julian, I added some new functionality where if you click on a reference (eg. [24]), it will go to the appropriate reference. Also, table of content links should also go to the correct place EVEN if you use tabs or accordion style. I also fixed a strict warning bug (calling a method statically, even though it wasn't declared statically.)

I still don't understand 4 though.

Please try again?

Best, Loong

JulianSMoore commented 9 years ago

Hi Loong

Thanks for the update; I've been working through the changes

Results (all with WP 2014 as active theme)

1/ On install, plugin failed to activate 1st time - but activated successfully from the plugins area of admin immediately afterwards.

2/ I'm afraid goto references does not work with either tabs or accordion; the references are a top level item and by definition can't be active at the same time as any other top level element.

3/ With contents box and tabs, I wasn't able to reproduce your success on goto from contents: click on content link only worked if the target was in the active tab

4/ (contents box on, no tabs or accordion) Click on reference mark [n] does go to the footnote position reference, but clicking the up arrow to return doesn't work; I received "This url does not meet the site security guidelines". Now, of course I have commented out the "own site" check, but there is separate checking in pass_url_check...

The whitelist urls that appear in pass_url (NB global white list is empty - where might that list come from?) are:

http://blog.pantologea.com/Kumu/ http://en.wikipedia.org/

when testing the (incorrect) return URL http://blog.pantologea.com/uncategorized/kumu-process-contents-normal/

The footnote return is failing because the URL in get_wiki_content is a blog url being checked against the "wiki" whitelist.

The return URL embedded in the embedded wiki page is actually http://blog.pantologea.com?wikiembed-url=http%3A%2F2Fblog.pantologea.com%2Funcategorized%2Fkumu%2Dprocess%2Dcontents%2Dnormal%2F&wikiembed-title=%2191#cite_ref-1, before it appears in get_wiki_content as http://blog.pantologea.com/uncategorized/kumu-process-contents-normal/; the proper target seems to have been lost as well.

[Don't know whether its appropriate to suggest this, but would there be an advantage in using strcasecmp instead of "==" in pass_url_check to make it case insensitive?]

6/ The contents box, when displayed, could do with a small bottom margin- though there is a small gap when it appears above tabs, the bottom of the box is right agains the top of the heading (if no tabs or accordion) or the top item of an accordion list.

7/ Re wiki "red links". I have attached an image that shows the words "observable" as blue and "dog" as read because: "Observable" exists as a wikipedia page but dog does not (yet).

The "Observable" link is http://blog.pantologea.com/Kumu/Observable - i.e. directly to an existing article,

The "Dog" link is http://blog.pantologea.com/Kumu/index.php?title=Dog&action=edit&redlink=1

Note that the latter directs to index.php with action=edit and "redlink" status (i.e. article non-existent flag) - it's this should be trapped: if a user clicks on a "redlink" (i.e. a link with "redlink=1" in the URL, nothing should happen; this would of course require that links in the wiki article are scanned and if they contain "redlink=1" they should (probably) be converted to plain text, i.e. hyperlink removed. On mouseover the redlink shows "Dog (page does not exist)", but this is not shown in the image.

I hope that helps and thanks again for the attention,

Best regards

Julian

redlink

brabbins commented 9 years ago

Just a FYI. We are still actively working on this issue.

JulianSMoore commented 9 years ago

I've been checking back daily to see if there was news so that's v. good to know! Though activity to date was encouraging not knowing the status of the plugin I didn't know what to expect. It's one of those things that just soooo should exist it would have been sad if it was succumbing to entropy ;)

Thx^2

PS I hate the button labels here - hence the incorrect close.