Page MenuHomePhabricator

TypeError (trying to get the value of a null attribute) when trying to edit the page list of a specific ProofreadPage index page
Closed, ResolvedPublicBUG REPORT

Description

Steps to Reproduce:

Actual Results:

Expected Results:

  • The page list opens.

Event Timeline

Soda subscribed.

Will take a look.

@Tacsipacsi Is it common practice on hu.wikisource to use the empty page number name to denote pages that don't have any page number ?

There are 19 index pages on huwikisource, so there are no really established practices yet. I found two pages containing empty in the index namespace. Probably we could change them, but the software shouldn’t fail on this anyway, especially as this is a “magic value” interpreted by the extension (supported since d1b07f8f49a6 committed in March 2009).

Change 630374 had a related patch set uploaded (by Sohom Datta; owner: Sohom Datta):
[mediawiki/extensions/ProofreadPage@master] Fix JS error due to the page number type having value "empty"

https://gerrit.wikimedia.org/r/630374

There are 19 index pages on huwikisource, so there are no really established practices yet. I found two pages containing empty in the index namespace. Probably we could change them, but the software shouldn’t fail on this anyway, especially as this is a “magic value” interpreted by the extension (supported since d1b07f8f49a6 committed in March 2009).

Nah, your right, it shouldn't be erroring out.

That being said, it's going to take a non-trivial amount of time/work to rework the widget to support the empty magic value (since the widget mostly depends on all page links being visible). Given that most wikisources use the "-" value to denote empty page numbers and the magic value itself is not used much, I'm not sure it would be useful to add full support for the empty magic value in the widget. I think it would be better/easier if the Index pages were modified to use "-".

If the widget depends so much on links, simply silencing the issue will probably make it even worse by shifting indices, not processing pages etc. But does it really depend on the links being <a> tags? What if the PHP code emits these pages as

<span data-proofreadpage-title="Oldal:Magyar remekírók - A magyar irodalom főművei - 54. kötet - Magyar népdalok.djvu/1"><span style="visibility:hidden;">00</span>1</span>

which also contains all the necessary information? (I don’t know how the PHP code works, but it should be possible.)

If you can’t deal with non-<a> pages, the widget should watch for empty pages and deny to open if it finds one.

I'm going to go for explicitly failing to open the widget when it finds the empty value in a pagelist.

It's not that it's not able to detect the <span> tags, but there are quite a lot of assumptions that we make regarding the pagelist many of which may/may not hold true when the empty tag is supported (length of the pagelist is equal to the number of pages in the book, scan number of the page also corresponds to the relative numbering in the button preview). All this is without considering whether or not adding the span tags will mess up parsing the pagelists for existing gadgets.

The addition of some of the other builtin systems are tracked in T263651. We should summarise this discussion there so that we can take a look at the empty page numbering system later and try to figure out a better solution.

Change 630374 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] Disallow the PagelistWidget from loading when the "empty" value is used

https://gerrit.wikimedia.org/r/630374