564791 - template.XULMethInc() and friends have wrong target ( : instead of / )

Status: RESOLVED FIXED

(developer.mozilla.org Graveyard :: User management, task)

Description

[Ben Bucksch (:BenB)]

XUL:Method:addProgressListener
#REDIRECT [[en/XUL/Method/addProgressListener]]

{{template.XULMethInc("addTabsProgressListener")}} 

Reproduction:
1. Go to <https://developer.mozilla.org/en/XUL/tabbrowser>, look for addTabsProgressListener definition. It's not there, it's a broken template include.
2. <https://developer.mozilla.org/index.php?title=en/XUL/tabbrowser&action=edit>
has:
{{template.XULMethInc("addProgressListener")}} 
{{template.XULMethInc("addTabsProgressListener")}} 
but addProgressListener is included, while addTabsProgressListener is not.
3. / is superceded by : , via redirects:
   <https://developer.mozilla.org/index.php?title=en/XUL:Method:addProgressListener&redirect=no> redirects to <https://developer.mozilla.org/en/XUL/Method/addProgressListener>, that's why that works.
4. <https://developer.mozilla.org/en/XUL/Method/addTabsProgressListener> exists, <https://developer.mozilla.org/en/XUL:Method:addTabsProgressListener> does not.

So, in short: the template.XULMethInc() template refers to "XUL:Method:", although that is deprecated and "XUL/Method/" is now correct, but the template has not been updated. Please do so.
It only happens to work, because all the old methods have redirects and the include follows the redirect. Newly added properties do not have the redirect, though, unless explicitly added, so the template is indeed broken.

Comment 1

[Eric Shepherd [:sheppy]]

Fixed. This template is now intelligent and builds the URL on the fly, checking to see which version of the URL to use on a case-by-case basis based on which one exists.

Comment 2

[Ben Bucksch (:BenB)]

Thanks for the switf fix.

Unfortunately, this seems to have broken the templates. Neither addProgressListener nor addTabsProgressListener are included now.
Please compare
old: <http://mdn.beonex.com/en/XUL/tabbrowser#m-addTabsProgressListener>
now: <https://developer.mozilla.org/en/XUL/tabbrowser#m-addTabsProgressListener>

Comment 3

[Eric Shepherd [:sheppy]]

Fixed.

Comment 4

[Eric Shepherd [:sheppy]]

Oops, no, now I see the real problem. Mis-remembered what this template is supposed to do. :)

Comment 5

[Eric Shepherd [:sheppy]]

OK, now this seems to really be fixed.

Comment 6

[Ben Bucksch (:BenB)]

Please try the page above. The anchors are still broken.

Comment 7

[Eric Shepherd [:sheppy]]

They look correct to me. Where are you seeing a problem?

Comment 8

[Ben Bucksch (:BenB)]

Does <http://mdn.beonex.com/en/XUL/tabbrowser#m-addTabsProgressListener>, and the link in the method overview, jump to the definition? Not for me.
(Well, the page doesn't load at all for me. It didn't when I posted this.)

Some due diligence, please.

Note that the same applies to attributes and properties, of course.

Comment 9

[Ben Bucksch (:BenB)]

> the page doesn't load at all for me

Now it does, still doesn't work.

Comment 10

[Eric Shepherd [:sheppy]]

Loading off a clone of MDC isn't going to necessarily produce correct results. It works when run off MDC itself. Fixing it so it works off your copy of MDC is not really very high priority. :)

Comment 11

[Ben Bucksch (:BenB)]

> Does <http://mdn.beonex.com/en/XUL/tabbrowser#m-addTabsProgressListener>, and

Sorry, I meant <https://developer.mozilla.org/en/XUL/tabbrowser#m-addTabsProgressListener> of course. (I did use developer.mozilla.org in the browser, just pasted the wrong URL in the comment).

Problem persists. Would you PLEASE finally try it yourself whether it works? That's normal when you change something, that you test everything that is likely to be affected, whether it works. This clearly and obviously doesn't, and you could trivially see yourself instead of letting me REOPEN the bug all the time. And please try all aspects.

Comment 12

[Eric Shepherd [:sheppy]]

Attachment: Showing this working correctly

As I've already said, I've already tested this and it's working fine. Attached is a screen shot showing your exact example working correctly. Please stop reopening this unless you can find an actual problem.

Comment 13

[Ben Bucksch (:BenB)]

Your screenshot shows that the template is included, but the anchor (comment 6) is still broken.
Things to test (all totally obvious):
* Overview
  List at top of page shows
  "Methods  addProgressListener, addTab, addTabsProgressListener, ..."
  WORKS (comment 2: you broke that here, but then fixed it again)
* Anchor
  A click on an entry in the above mentioned overview scrolls the page
  to the below mentioned definition.
  BROKEN (comment 6)
* Definition on same page
  "addProgressListener
    Return type: no return value
    Add a progress listener to the b..."
  The header must link to the below mentioned page.
  WORKS (that was the original bug in comment 0, but you fixed that)
* Definition on separate page
  (Link to)
  <https://developer.mozilla.org/En/XUL/Method/addTabsProgressListener>
  WORKS (that was the original bug in comment 0, but you fixed that)

All of the above must work for:
* methods
* properties
* elements

All of that must for work:
* XUL
* DOM
* whereever else : changed to / and templates are used

Now, I spent half an hour (!) explaining a problem that can be seem within 30 seconds by just clicking around a bit.

Comment 14

[Eric Shepherd [:sheppy]]

You could have fixed it yourself by now instead of being annoying about it. If it were obvious, I'd have fixed it already. This is all I've worked on for the past two hours, trying to make you happy.

I'm working on it some more.

Comment 15

[Ben Bucksch (:BenB)]

Thanks for looking into it.
But this is not about making me happy, but about fixing a *bug* in MDC.
And the templates are total gibberish to me, so I can't fix them.

Comment 16

[Ben Bucksch (:BenB)]

> And the templates are total gibberish to me

Well, it was. Your new version looks *much* better.
<https://developer.mozilla.org/index.php?title=Template:XULMethInc&action=diff&revision=7&diff=13>
Nice work.
I still don't understand enough of the templates in general to be able to fix it.

Comment 17

[Ben Bucksch (:BenB)]

All I can see is that the old code used wiki.template() but the new uses wiki.page(). I don't know enough to know why or whether that's causing the new bug.

Comment 18

[Eric Shepherd [:sheppy]]

OK, I think this is fixed now. The anchor seems to be working correctly, and it all looks right. Please let me know if anything else is wrong.

Comment 19

[Ben Bucksch (:BenB)]

Thanks, that's much better.
Thank you also for fixing the code, it's *much* more readable now.

Could you, or somebody else, please also fix the other instances of the same problem?

Comment 13:
> ...
> All of the above must work for:
> * methods
> * properties
> * elements
>
> All of that must for work:
> * XUL
> * DOM
> * whereever else : changed to / and templates are used

Comment 20

[Ben Bucksch (:BenB)]

Another way to fix this bug would be to simply
* move all pages from ":" to "/"
* Change templates to use "/" instead of ":" (instead of both, as now)
* Use an Apache URL rewrite rule to redirect links to ":" URLs to "/" URLs
  (instead of wiki #REDIRECT on each and every ":" page)

Comment 21

[Eric Shepherd [:sheppy]]

Yes, that's the ultimate goal. But right now, I don't think anyone has the time or patience available to move all those pages one at a time, so this is a temporary workaround.

I've requested that MindTouch add a feature to let you do page moves using a title regexp, which would make this much easier.

Comment 22

[Eric Shepherd [:sheppy]]

OK, this is now fixed for XUL attributes, properties, and methods.

The corresponding DOM templates had previously been fixed; if you happen to find one that's still broken, let me know. I can't find any though. Some are still written the old way but still work, so for now I'm leaving them alone.

Comment 23

[Ben Bucksch (:BenB)]

> move all those pages one at a time

Yes, that's what I meant: Do it all at once, using a script.

> OK, this is now fixed for XUL attributes, properties, and methods.

Thanks!

Comment 24

[Ben Bucksch (:BenB)]

> I've requested that MindTouch add a feature to let you do page moves
> using a title regexp

Good, thanks