Make WordPress Core

Opened 10 years ago

Last modified 3 months ago

#31711 new defect (bug)

is_front_page flag affected by frontpage ID and page Title strange conflict

Reported by: m_i_n's profile m_i_n Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.1
Component: General Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

I want to report a strange bug. The is_front_page flag has incorrect value in cases like described below:

Front page in Settings / Reading is set to a static page: http://i.imgur.com/DzrbOVM.png
This Sample Page ID is 2
If you visit any other page, all is fine. For example the Lorem ipsum page: http://i.imgur.com/n5M0x8R.png
However, if you change title of this page to be the same as ID of a frontpage - in my case 2, then the is_front_page flag changes value to incorrect: http://i.imgur.com/KCa1JYo.png

It happens for titles like: 2, 02, 000002. Basically always if (int)$page_title == $front_page_id

The wrong is_front_page flag may affect themes/plugins which use it.

It's quite rare for the bug to take effect, but it happens, as I got a report from a user of my theme.

Attachments (1)

31711.patch (1.6 KB) - added by tyxla 10 years ago.
Fixing is_front_page() conflicts with the titles of other page when they are the same like the ID of the front page. Including a unit test for that case.

Download all attachments as: .zip

Change History (11)

#1 @tyxla
10 years ago

  • Keywords needs-patch added

That's such an interesting bug.

This is caused by the (expected) ambiguous behavior of the is_page() conditional tag, which is used within is_front_page().

As the documentation states, is_page() accepts: Page ID, title, slug, path, or array of such, so in that case 2 is being accepted as title, which is why the is_front_page() function returns true.

Patch with a fix is coming in a minute.

Version 0, edited 10 years ago by tyxla (next)

@tyxla
10 years ago

Fixing is_front_page() conflicts with the titles of other page when they are the same like the ID of the front page. Including a unit test for that case.

#2 @tyxla
10 years ago

  • Keywords has-patch added; needs-patch removed

#3 @DrewAPicture
10 years ago

#31009 was marked as a duplicate.

#4 @MikeHansenMe
10 years ago

Tested this and it worked as expected. Unit test also passes.

#5 @DrewAPicture
10 years ago

  • Version changed from trunk to 3.1

This ticket was mentioned in PR #7101 on WordPress/wordpress-develop by @tw2113.


4 months ago
#6

  • Keywords has-unit-tests added

This code change updates original changes from the trac ticket below, to match the current code from WordPress Trunk. Everything should match what the original patch submitter added, with the exception of strict type checking between integers.

Trac ticket: See https://core.trac.wordpress.org/ticket/31711

@mukesh27 commented on PR #7101:


4 months ago
#7

@tw2113 The unit tests going failed, could you please check it?

@tw2113 commented on PR #7101:


4 months ago
#8

Still checking on the unit test portion.

@tw2113 commented on PR #7101:


4 months ago
#9

Notes for future me:

7) Tests_Query_Conditionals::test_page_on_front
is_front_page is false but is expected to be true.

/var/www/tests/phpunit/includes/abstract-testcase.php:1145
/var/www/tests/phpunit/tests/query/conditionals.php:49
/var/www/vendor/bin/phpunit:122

8) Tests_Query_Conditionals::test_is_front_page_id_equals_the_title_of_other_page
Failed asserting that false is true.

/var/www/tests/phpunit/tests/query/conditionals.php:1321
/var/www/vendor/bin/phpunit:122

9) Tests_Query_VerbosePageRules::test_page_on_front
is_front_page is false but is expected to be true.

/var/www/tests/phpunit/includes/abstract-testcase.php:1145
/var/www/tests/phpunit/tests/query/conditionals.php:49
/var/www/vendor/bin/phpunit:122

10) Tests_Query_VerbosePageRules::test_is_front_page_id_equals_the_title_of_other_page
Failed asserting that false is true.

/var/www/tests/phpunit/tests/query/conditionals.php:1321
/var/www/vendor/bin/phpunit:122

@tw2113 commented on PR #7101:


3 months ago
#10

@mukeshpanchal27 This is ready for re-review after fixing strict type checking.

Thanks.

Note: See TracTickets for help on using tickets.