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

XML Site Map Bugs #1554

Closed
luxurypropertyagent opened this Issue Feb 11, 2018 · 19 comments

Comments

Projects
None yet
5 participants
@luxurypropertyagent

luxurypropertyagent commented Feb 11, 2018

XML sitemap bugs:

  1. When adding another url
    Example: Wordpress site location www.page.com/blog/
    You would like to add URL www.page.com
    appears on xml sitemap as: www.page.com/blog/www.page.com
@michaeltorbert

This comment has been minimized.

Member

michaeltorbert commented Feb 11, 2018

One way around this could be for AIOSEOP to check if it's an absolute URL entered, and if that's the case, not treat it as a relative URL.

@contactashish13

This comment has been minimized.

Contributor

contactashish13 commented Feb 23, 2018

I believe the additional pages feature is supposed to work for both relative as well as absolute URLs. By definition, absolute URLs should be specified with the scheme. I think if we add a comment to the section that tells users to prefix absolute URLs with http(s):// , it should remove the confusion and the bug.

@wpsmort wpsmort self-assigned this Mar 13, 2018

@wpsmort

This comment has been minimized.

Member

wpsmort commented Mar 13, 2018

I've opened a separate issue for updating the inline help and documentation - #1604

@wpsmort

This comment has been minimized.

Member

wpsmort commented Mar 13, 2018

@contactashish13 We still need to fix this so that our code checks whether an absolute URL is being entered. i.e. if test.com or www.test.com is entered then this is an absolute URL and we should treat it as such in the sitemap.

@contactashish13

This comment has been minimized.

Contributor

contactashish13 commented Mar 14, 2018

@wpsmort if the URL entered is of the below format, both will be considered absolute because of the presence of a dot before the first /. I think it would be best to force users to add the protocol no matter what. What do you think?

  1. this.is.com/blah/blah/blah (absolute)
  2. this.is.a/test.page/ (relative)

@michaeltorbert michaeltorbert modified the milestones: 2.4.6, 2.4.7 Mar 15, 2018

@michaeltorbert

This comment has been minimized.

Member

michaeltorbert commented Mar 19, 2018

force users to add the protocol

Agreed.
The only issue now is how to handle values already in the database.

@contactashish13

This comment has been minimized.

Contributor

contactashish13 commented Mar 19, 2018

we can assume that presence of a dot before the first / indicates absolute URLs?

@michaeltorbert

This comment has been minimized.

Member

michaeltorbert commented Mar 19, 2018

well we could probably just use something built in like wp_http_validate_url() and reject it if it doesn't pass... still not sure how we'd handle the stuff already in the db

@contactashish13

This comment has been minimized.

Contributor

contactashish13 commented Mar 19, 2018

for stuff already in the db, if it does not pass wp_http_validate_url() then we assume that presence of a dot before the first / indicates absolute URLs and we add the protocol/scheme

@michaeltorbert

This comment has been minimized.

Member

michaeltorbert commented Mar 19, 2018

I hate to make changes to the db like that, or do you mean do it on the fly before output?

@contactashish13

This comment has been minimized.

Contributor

contactashish13 commented Mar 19, 2018

yes, on the fly

  1. when the sitemap is being shown
  2. when the settings are being shown
@michaeltorbert

This comment has been minimized.

Member

michaeltorbert commented Mar 19, 2018

When the settings are being shown will be a tough one, and maybe isn't really necessary.

@contactashish13

This comment has been minimized.

Contributor

contactashish13 commented Aug 21, 2018

@wpsmort this is as per #1554 (comment)

@contactashish13 contactashish13 removed their assignment Aug 21, 2018

contactashish13 added a commit to contactashish13/all-in-one-seo-pack that referenced this issue Aug 21, 2018

@wpsmort

This comment has been minimized.

Member

wpsmort commented Aug 21, 2018

@contactashish13 To clarify, my comment #1554 (comment) was a question. As I'm not familiar with wp_http_validate_url() I was asking if it was possible to allow relative URLs or not.

I tested the PR and found two instances where I can enter invalid URLs:

1). Enter the URL without slashes, i.e. http:test.com and hit enter instead of clicking on the Add URL button

2). Enter the URL with only one slash, i.e. http:/test.com and hit enter instead of clicking on the Add URL button

In both cases it adds the URL and it also seems to wipe any other URLs from the database. Let me know if you cannot reproduce this.

contactashish13 added a commit to contactashish13/all-in-one-seo-pack that referenced this issue Aug 23, 2018

contactashish13 added a commit to contactashish13/all-in-one-seo-pack that referenced this issue Aug 23, 2018

@contactashish13

This comment has been minimized.

Contributor

contactashish13 commented Aug 23, 2018

@wpsmort I feel it does not make sense to allow users to input relative URLs when the sitemap ought to contain absolute URLs only.

As for URLs already existing in the database, I have tried to deduce whether they are absolute or relative. Can you please try a few random examples and let me know if the logic fails somewhere?

I have fixed the issue you reported above.

@contactashish13 contactashish13 removed their assignment Aug 23, 2018

@wpsmort

This comment has been minimized.

Member

wpsmort commented Aug 27, 2018

@contactashish13 I tested all variations of absolute and relative URLs. The only ones that were a problem for URLs already in the database were these examples:

http:domain.com - This was output as http://aioseop1554.local/domain.com
http:/domain.com - This was output as http://aioseop1554.local/domain.com

Is there anything you can do to correctly sanitize these when they are output?

contactashish13 added a commit to contactashish13/all-in-one-seo-pack that referenced this issue Aug 27, 2018

@contactashish13 contactashish13 removed their assignment Aug 27, 2018

@contactashish13

This comment has been minimized.

Contributor

contactashish13 commented Aug 27, 2018

@wpsmort added support for these types as well

@wpsmort wpsmort added Bug and removed Needs Testing labels Aug 27, 2018

contactashish13 added a commit to contactashish13/all-in-one-seo-pack that referenced this issue Aug 29, 2018

michaeltorbert added a commit that referenced this issue Sep 6, 2018

XML Site Map Bugs #1554 (#1830)
* Issue comments

1. make scheme in URL mandatory on input
2. for URLs already present without a scheme, smartly make them valid URLs

* use enter key to circumvent validation

#1554 (comment)

* use enter key to circumvent validation

* PR comments

#1554 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment