Last Comment Bug 747412 - Installing Favimon does not show the right parenthesis on the desktop shortcut or start menu
: Installing Favimon does not show the right parenthesis on the desktop shortcu...
Status: VERIFIED FIXED
[marketplace-beta=]
:
Product: Firefox Graveyard
Classification: Graveyard
Component: Web Apps (show other bugs)
: unspecified
: All All
: -- normal
: Firefox 15
Assigned To: Ed Lee :Mardak
: Jason Smith [:jsmith]
:
Mentors:
: 748676 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-20 08:42 PDT by Jason Smith [:jsmith]
Modified: 2016-02-04 15:00 PST (History)
7 users (show)
jsmith: in‑moztrap-
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Favimon Parentheses Issue (864.67 KB, image/png)
2012-04-20 09:32 PDT, Jason Smith [:jsmith]
no flags Details
v1 (1.12 KB, patch)
2012-04-26 16:53 PDT, Ed Lee :Mardak
felipc: review+
Details | Diff | Splinter Review

Description Jason Smith [:jsmith] 2012-04-20 08:42:44 PDT
Steps:

1. Go to https://apps.mozillalabs.com/appdir/
2. Install Favimon

Expected:

The application should be installed natively. The desktop shortcut and the item in the start menu should be Favimon (beta).

Actual:

The application is installed natively. However, the desktop shortcut and the item in the start menu is listed as Favimon (beta.
Comment 1 Jason Smith [:jsmith] 2012-04-20 09:32:33 PDT
Created attachment 617004 [details]
Favimon Parentheses Issue
Comment 2 Tim Abraldes [:TimAbraldes] [:tabraldes] 2012-04-20 14:25:36 PDT
This is related to the sanitization code in bug 704574.
Comment 3 Jason Smith [:jsmith] 2012-04-25 00:40:07 PDT
*** Bug 748676 has been marked as a duplicate of this bug. ***
Comment 4 Ed Lee :Mardak 2012-04-26 14:29:53 PDT
The trailing ) is removed by WebappsInstaller.jsm's stripStringForFilename:

  let stripBackRE = new RegExp("\\W*$","gi");
Comment 5 Ed Lee :Mardak 2012-04-26 14:36:32 PDT
felipe, any particular reason trailing characters need to be stripped? Stripping from the beginning makes more sense if the first character must be alphanumeric, but usually there's no requirement for non-first characters.. ?
Comment 6 Ed Lee :Mardak 2012-04-26 14:37:14 PDT
This also affects os x in /Applications and Launcher.
Comment 7 :Felipe Gomes (needinfo me!) 2012-04-26 14:46:43 PDT
Nope, the original idea was to strip blank/invisible characters, but this ended up to be too aggressive
Comment 8 Ed Lee :Mardak 2012-04-26 15:21:55 PDT
So probably just change it to stripping \s ?
Comment 9 Ed Lee :Mardak 2012-04-26 16:53:26 PDT
Created attachment 618859 [details] [diff] [review]
v1
Comment 10 Jason Smith [:jsmith] 2012-04-26 20:36:31 PDT
(In reply to Edward Lee :Mardak from comment #9)
> Created attachment 618859 [details] [diff] [review]
> v1

FYI - Looking at the patch comment:

//strip everything from the front up to the first [0-9a-zA-Z]

This isn't localized to languages outside of the united states. This explains why bug 733482 might be happening. This needs to be localized, along with other regex expressions used for manifest parsing.
Comment 12 :Ms2ger (⌚ UTC+1/+2) 2012-04-27 06:49:36 PDT
https://hg.mozilla.org/mozilla-central/rev/add831dc812e

Note You need to log in before you can comment on or make changes to this bug.