Skip to content

Fixes tabs for geo-location RSS feeds#13801

Merged
kraftbj merged 4 commits intomasterfrom
fix/12051-georss-whitespacing
Oct 25, 2019
Merged

Fixes tabs for geo-location RSS feeds#13801
kraftbj merged 4 commits intomasterfrom
fix/12051-georss-whitespacing

Conversation

@ChaosExAnima
Copy link
Copy Markdown
Contributor

@ChaosExAnima ChaosExAnima commented Oct 22, 2019 •
edited
Loading

Fixes #12051

Changes proposed in this Pull Request:

  • This resolves an issue where the RSS feeds with geo-location had invalid whitespacing, causing parser issues.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • This is a bug fix.

Testing instructions:

  • Visit RSS feed, and view namespaces.

Before:
Screenshot from 2019-10-22 11-51-52

After:
Screenshot from 2019-10-22 11-50-44

Proposed changelog entry for your changes:

  • Fixed spacing for RSS geo-location namespaces.

@ChaosExAnima ChaosExAnima self-assigned this Oct 22, 2019
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Oct 22, 2019 •
edited
Loading

Warnings
⚠️

pre-commit hook was skipped for one or more commits

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 731198c

@ChaosExAnima ChaosExAnima requested a review from a team October 22, 2019 15:49
ChaosExAnima
ChaosExAnima commented Oct 22, 2019
View reviewed changes
modules/geo-location/class.jetpack-geo-location.php Outdated

This comment was marked as outdated.

Sign in to view

@ChaosExAnima ChaosExAnima force-pushed the fix/12051-georss-whitespacing branch from 0e24d21 to 5292d0f Compare October 22, 2019 15:51
@ChaosExAnima ChaosExAnima added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Oct 22, 2019
ChaosExAnima
ChaosExAnima commented Oct 22, 2019
View reviewed changes
modules/geo-location/class.jetpack-geo-location.php
public function rss_namespace() {
echo PHP_EOL . 'xmlns:georss="http://www.georss.org/georss" xmlns:geo="http://www.w3.org/2003/01/geo/wgs84_pos#"' . PHP_EOL;
echo PHP_EOL . "\t" . 'xmlns:georss="http://www.georss.org/georss\"';
echo PHP_EOL . "\t" . 'xmlns:geo="http://www.w3.org/2003/01/geo/wgs84_pos#"';
Copy link
Copy Markdown
Contributor Author

@ChaosExAnima ChaosExAnima Oct 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split apart for readability.

Copy link
Copy Markdown

@Ael02 Ael02 Mar 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix/12051-georss-whitespacing

@ChaosExAnima ChaosExAnima added this to the 7.9 milestone Oct 22, 2019
@jeherve jeherve added the Enhancement Changes to an existing feature — removing, adding, or changing parts of it label Oct 22, 2019
jeherve
jeherve requested changes Oct 22, 2019
View reviewed changes
@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Oct 22, 2019
@ChaosExAnima @jeherve
Update modules/geo-location/class.jetpack-geo-location.php
495ec05 
Co-Authored-By: Jeremy Herve <jeremy@jeremy.hu>
@ChaosExAnima ChaosExAnima added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Oct 22, 2019
jeherve
jeherve previously approved these changes Oct 23, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, it should be good to merge!

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Oct 23, 2019
kraftbj
kraftbj reviewed Oct 24, 2019
View reviewed changes
modules/geo-location/class.jetpack-geo-location.php Outdated
*/
public function rss_namespace() {
echo PHP_EOL . 'xmlns:georss="http://www.georss.org/georss" xmlns:geo="http://www.w3.org/2003/01/geo/wgs84_pos#"' . PHP_EOL;
echo PHP_EOL . "\t" . 'xmlns:georss="http://www.georss.org/georss\"';
Copy link
Copy Markdown
Contributor

@kraftbj kraftbj Oct 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the trailing backslash needed?

Copy link
Copy Markdown
Contributor Author

@ChaosExAnima ChaosExAnima Oct 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, good catch! Fixed in 731198c

@kraftbj
Copy link
Copy Markdown
Contributor

kraftbj commented Oct 25, 2019

Test failure looks like an unrelated fragile sync test that failed on master/php7.4. Rerunning.

@kraftbj kraftbj merged commit c97bfc3 into master Oct 25, 2019
@kraftbj kraftbj deleted the fix/12051-georss-whitespacing branch October 25, 2019 14:22
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Oct 25, 2019
jeherve added a commit that referenced this pull request Oct 28, 2019
@jeherve
Changelog: add #13801
c4294ed
jeherve added a commit that referenced this pull request Oct 29, 2019
@jeherve
7.9: Changelog (#13608)
6cd1621 
* 7.9: Changelog

* Update version number

* Update stable tag and tested up to

* Changelog: add #13530

* changelog: add #13578

* Changelog: add #13598

* Changelog: add entry for numerous block preview changes

* Changelog: add #13599

* changelog: add #13541

* Changelog: add #13542

* Changelog: add #13331

* Changelog: add #13558

* Changelog: add #13409

* Changelog: add #13582

* Changelog: add #13600

* Changelog: add #13601

* Changelog: add #13595

* Changelog: add #12695

* Changelog: add #13009

* Changelog: add #13649

* Changelog: add #13450

* Changelog: add #13507

* Changelog: add #13658

* Changelog: add #13687

* changelog: add #13683

* Changelog: add #9323

* Changelog: add #13681

* Fix typos in readme

* Add link to WordPress Beta Tester plugin

* Changelog: add #13630

* Changelog: add #13695

* Changelog: add #13659

* Changelog: add #13716

* Changelog: add #13664

* Changelog: add #13682

* Changelog: add #13362

* Changelog: add #13563

* Add testing list for #13563

* Changelog: add #13735

* Changelog: add #13752

* Changelog: add #13624

* Changelog: add #13756

* Changelog: add #13745

* Changelog: add #13728

* Changelog: add #13779

* Changelog: add #13699

* Changelog: add #13804

* Changelog: add #13761

* Changelog: add #13637

* Changelog: add #13517

* Changelog: add #13521

* Changelog: add #13729

* Testing list: add testing instructions for #13729

* Changelog: add sync changes

* Changelog: add #13807

* Changelog: add #13654

* Changelog: add #13795

* Changelog: add #13801

* Changelog: add #13818

* Changelog: add #13725

* Changelog: add #13831

* Changelog: add #13516

* Testing list: add Twenty Twenty instructions

* Changelog: add #13799

* Changelog: add #13805

* Changelog: add #13688

* Changelog: add #13830
@Automattic Automattic deleted a comment from Ael02 Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kraftbj kraftbj kraftbj approved these changes

@jeherve jeherve jeherve left review comments

+1 more reviewer

@Ael02 Ael02 Ael02 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@ChaosExAnima ChaosExAnima

Labels

Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Geo Location

Projects

None yet

Milestone

7.9

Development

Successfully merging this pull request may close these issues.

Geo-location: improve display of xmlns:georss attribute

6 participants

@ChaosExAnima @jetpackbot @kraftbj @jeherve @Ael02 @matticbot