Closed Bug 1146683 Opened 9 years ago Closed 9 years ago

The php_url helper function is broken

Categories

(www.mozilla.org :: Bedrock, defect)

Product:
www.mozilla.org
Component:
Bedrock
Version:
Production
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: malexis, Unassigned, Mentored)

References

Details

(Keywords: regression, Whiteboard: [kb=1700343] [good first bug]) 

http://www.mozilla.org/en-US/MPL is a 404. Need to remove the link from the body text on this page (under the 4th question): http://www.mozilla.org/en-US/foundation/trademarks/faq/
Whiteboard: [good first bug] → [kb=1700343] [good first bug]
(In reply to Jennifer Bertsch [:jbertsch] from comment #1)
> https://www.mozilla.org/MPL/ works.

OK edited summary to edit the link instead of remove.
Summary: Remove MPL hyperlink from /foundation/trademarks/faq → Add trailing slash to MPL hyperlink on /foundation/trademarks/faq
Hello, I would like to work on this bug, though am unsure where I would go to begin editing the link. I am new to this, sorry if this seems like to basic of a question.
Hi Wickie, welcome to mozilla.org! The relevant code is here:

https://github.com/mozilla/bedrock/blob/master/bedrock/foundation/templates/foundation/trademarks/faq.html#L54

But... looks like this link itself is correct. The issue is probably that the php_url function is not working as expected for some reason; it should not add the en-US (or any other locale) prefix to the link:

https://github.com/mozilla/bedrock/blob/master/bedrock/mozorg/helpers/misc.py#L38-L47

Do you have a Python experience? The detailed document can be found at:

http://bedrock.readthedocs.org/en/latest/
Component: Pages & Content → Bedrock
Keywords: regression
OS: Mac OS X → All
Hardware: x86 → All
Summary: Add trailing slash to MPL hyperlink on /foundation/trademarks/faq → The php_url helper function is broken
Version: Development/Staging → Production
Alright, so I am probably going to say something stupid now. Is it wrong for me to explicitly make php_url skip joining the locale if the url is /MPL?
For now only /MPL is obviously broken, but looks like we have a similar issue for /thunderbird/. The /thunderbird/ page has a locale prefix, but if we append a locale prefix to the /thunderbird/ link, it will be broken in some locales (as reported in Bug 1033930.)

So, I think the quick solution here is making php_url simply return the passed url, not only for /MPL but for all links, without appending any locale prefix.
Or we could just avoid using the function and instead use a static link in HTML.
I would upload my patch. But I do not have the required permissions yet. So I need to be assigned to this bug first, if anyone can do that. Also, wouldn't returning it without the locale mess up other calls made to that function? I was just going to leave it as is, except when the url is "/MPL" return url as is
Sorry I'm late here! I have cleaned up legacy php_url usage in 
https://github.com/mozilla/bedrock/pull/2904

So now we only have /MPL/ and /thunderbird/
https://github.com/mozilla/bedrock/search?q=php_url

I think we can simply
* remove the php_url function from misc.py, and
* update the HTML templates to use static links
** <a href="{{ php_url('/thunderbird/') }}"> → <a href="/thunderbird/">
** mpl=php_url('/MPL') → mpl='/MPL/'
instead of fixing the function.

Wickie: do you want to send a pull request for that?
Flags: needinfo?(wickie.leejr)
Hello! 

First time here. Is there anything left to do for this bug? I would love to contribute!
@wickie are you still working on this?
@Kohei Apologies! I have been extremely busy lately, I would like to still work on it, however am unfamiliar with making pull requests, and am going to be going out of town in a couple of days. I am not sure I will have time for it in the days I have before I go, so Deepak can have it if he is still interested.
Flags: needinfo?(wickie.leejr)
@Kohei I am new here. To fix the bug should I fork the repo, make changes and then send pull request?
@Kohei how can I get started with this bug?
Commits pushed to master at https://github.com/mozilla/bedrock

https://github.com/mozilla/bedrock/commit/3f4838f3629a9735dea86f5f0f8c59f7a3676ce6
[fix bug 1146683] Remove php_url helper function.

https://github.com/mozilla/bedrock/commit/83933474cb117519fb72b97ec0ec8b62d11a8b4a
Merge pull request #3050 from ishanroy/master

Fix Bug 1146683 - The php_url helper function is broken
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.