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

RESOLVED FIXED

Status

Mozilla Developer Network
User management
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: BenB, Unassigned)

Tracking

Details

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
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.
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.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Reporter)

Comment 2

8 years ago
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>
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Fixed.
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
Oops, no, now I see the real problem. Mis-remembered what this template is supposed to do. :)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
OK, now this seems to really be fixed.
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
(Reporter)

Comment 6

8 years ago
Please try the page above. The anchors are still broken.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
They look correct to me. Where are you seeing a problem?
(Reporter)

Comment 8

8 years ago
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.
(Reporter)

Comment 9

8 years ago
> the page doesn't load at all for me

Now it does, still doesn't work.
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. :)
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
(Reporter)

Comment 11

8 years ago
> 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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Created attachment 444465 [details]
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.
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
(Reporter)

Comment 13

8 years ago
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
(Reporter)

Comment 15

8 years ago
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.
(Reporter)

Comment 16

8 years ago
> 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.
(Reporter)

Comment 17

8 years ago
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.
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.
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
(Reporter)

Comment 19

8 years ago
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Updated

8 years ago
Summary: template.XULMethInc() has wrong target ( : instead of / ) → template.XULMethInc() and friends have wrong target ( : instead of / )
(Reporter)

Comment 20

8 years ago
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)
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.
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.
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
(Reporter)

Comment 23

8 years ago
> 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!
(Reporter)

Comment 24

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

Good, thanks
(Assignee)

Updated

5 years ago
Component: Administration → User management
Product: Mozilla Developer Network → Mozilla Developer Network
You need to log in before you can comment on or make changes to this bug.