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

Reduce the number of page duplicates in com_content categories when pagination is used. #12004

Closed
wants to merge 12 commits into from

Conversation

philip-sorokin
Copy link
Contributor

Summary of Changes

Reduce the number of page duplicates in com_content categories when pagination is used.

Testing Instructions

https://www.joomla.org/announcements.html?start=999999999999999999999 - it opens, while it should not. With this patch applied, pages will open if they really exist: https://www.joomla.org/announcements.html?start=390 - this will be the last page.

@zero-24
Copy link
Contributor

zero-24 commented Sep 10, 2016

@ADDONDEV can you fix the if syntax and add a space bettween the if and the openeing (

FILE: ...ld/joomla/joomla-cms/components/com_content/views/archive/view.html.php
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 86 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found
    |       | "if(...)\n...{...}\n...else\n"
--------------------------------------------------------------------------------
UPGRADE TO PHP_CODESNIFFER 2.0 TO FIX ERRORS AUTOMATICALLY
--------------------------------------------------------------------------------
FILE: ...d/joomla/joomla-cms/components/com_content/views/category/view.html.php
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 125 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found
     |       | "if(...)\n...{...}\n...else\n"
--------------------------------------------------------------------------------
UPGRADE TO PHP_CODESNIFFER 2.0 TO FIX ERRORS AUTOMATICALLY
--------------------------------------------------------------------------------
FILE: ...d/joomla/joomla-cms/components/com_content/views/featured/view.html.php
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 117 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found
     |       | "if(...)\n...{...}\n...else\n"
--------------------------------------------------------------------------------
UPGRADE TO PHP_CODESNIFFER 2.0 TO FIX ERRORS AUTOMATICALLY
--------------------------------------------------------------------------------

https://travis-ci.org/joomla/joomla-cms/jobs/159016449

@philip-sorokin
Copy link
Contributor Author

Can you test it, please?

@brianteeman
Copy link
Contributor

Is it the intended behaviour of this pr that ?start=999999999999999999999 gives a 404?


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/12004.

@philip-sorokin
Copy link
Contributor Author

Yes, it does.

@philip-sorokin
Copy link
Contributor Author

philip-sorokin commented Sep 20, 2016

The same behaviour you can see in the content_pagebreak plugin - if an article page does not exist, it gives a 404. Why not apply this approach to category blogs? When you have 200 pages, search engines index them, then you change the item per page number and 200 becomes 100. The problem is that the other 100 pages no longer exist, but they cannot be removed from search index without editing robots.txt/htaccess. I think it is very inconvenient.

@ghost
Copy link

ghost commented Jan 11, 2017

I have tested this item 🔴 unsuccessfully on 504c948

With or -out patch pages open. Example: Show 1 of 2 Pages, append in URL ?start=99999999 – always 2 (the last page) is shown.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12004.

@philip-sorokin
Copy link
Contributor Author

@franz-wohlkoenig

I use the RC demo pages (featured view). I set in the menu params the following configuration to make pagination:

Leading Articles - 1
Intro Articles - 1
Columns - 1
Links - 1

When I open page 4 that does not have any items, I get the 404. Could you, please, describe your testing instructions?

@ghost
Copy link

ghost commented Apr 11, 2017

@philip-sorokin Test Instructions shown in opened Comment.

@philip-sorokin
Copy link
Contributor Author

@franz-wohlkoenig
Can you give me a link to this comment. I am sorry, can't find it.

@ghost
Copy link

ghost commented Apr 11, 2017

on Top of this Thread, its your opening Comment about Issue.

@philip-sorokin
Copy link
Contributor Author

Still can't see any comments.

comment

@ghost
Copy link

ghost commented Apr 11, 2017

what i mean is:
bildschirmfoto 2017-04-11 um 07 34 24

@philip-sorokin
Copy link
Contributor Author

philip-sorokin commented Apr 11, 2017

@franz-wohlkoenig
This is my testing instructions, I thought you would write yours. Please, give me more details on the failure, because I tested it in 3 views successfuly:

-archive
-category
-featured

@ghost
Copy link

ghost commented Apr 11, 2017

I used your Instructions for Test. Have to retest, my Test is too long ago to remember.

@ghost
Copy link

ghost commented Apr 11, 2017

Test on "Archive". 9 archived Articles (5 en-GB, 4 All Lang.), Menu set on Lang. "en-GB".

1/4. Call menu, Pagination show 1/2 (expected):

1

2/4. Click Pagnation 2/2, Url change to index.php/en/archived-articles?start=5 (expected):

2

3/4. Change Url to index.php/en/archived-articles?start=6 (unexpected – Articles with Lang.: All are shown):

3

4/4. Change Url to index.php/en/archived-articles?start=9 (expected – 404

4

@philip-sorokin
Copy link
Contributor Author

And what is your conclusion? What about other views?

@wojsmol
Copy link
Contributor

wojsmol commented May 13, 2017

@philip-sorokin @franz-wohlkoenig don't expect this

3/4. Change Url to index.php/en/archived-articles?start=6 (unexpected – Articles with Lang.: All are shown):

@ghost
Copy link

ghost commented May 13, 2017

@wojsmol didn't get what you mean.

@wojsmol
Copy link
Contributor

wojsmol commented May 13, 2017

@franz-wohlkoenig See #12004 (comment) first question

@ghost
Copy link

ghost commented May 13, 2017

I have no Conclusion cause no Knowledge.

@philip-sorokin
Copy link
Contributor Author

I am closing this PR as it is not going to be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants