panels_page_add_page() returns a drupal_not_found(), if called with an invalid $layout parameter.
Why doesn't it simply fallback to "Choose layout", as if it was called without a $layout parameter? This would be more forgiving to the user and also four lines less code...
(While this is just a minor issue, I just didn't want to forget about it...)

Attached is a small patch changing this.

CommentFileSizeAuthor
#3 panels_page_add_page_3.patch1.2 KBpancho
panels_page_add_page.patch1023 bytespancho

Comments

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented
Status: Active » Needs review
merlinofchaos’s picture

merlinofchaos commented

In general, I prefer providing feedback for invalid arguments. I can go either way on this one, mind you, but it can actually be more confusing to get a page you were completely not expecting instead of an error message when an argument is invalid.

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented
StatusFileSize
new panels_page_add_page_3.patch1.2 KB

Okay, this should be the best approach: If the provided $layout is invalid, I file a warning and unset the layout.
This will become somewhat important as soon as panels_allowed_layouts() is implemented in panels_page.

sdboyer’s picture

sdboyer commented
Status: Needs review » Fixed

Well...since the use (or not) of panels_allowed_layouts is ENTIRELY up to the client module and it doesn't interfere in the normal editing processes, it won't make much of a difference here unless/until we rewrite panels_page to implement the allowed layouts system.

But, I think that in this case erring on the side of pushing the user to somewhere they might not expect is better than a 404, given that at this point, I think the problem much of Panels' user base is experiencing is NOT the one where they know exactly what to expect from Panels, then get surprised :)

Committed.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.