Closed Bug 1259227 Opened 4 years ago Closed 3 years ago
Exported profile's name is clipped after the first dot in file name
>>> My Info: Win7_64, Nightly 48, 32bit, ID 20160323030400 STR: 1. Open new tab 2. Open Devtools -> Performance 3. Press button "Start Recording Performance". Press it again to end recording. 4. Click "Save" near the recorded profile, save it with name: "2016.03.23 21-24-39 - profile recorded on gaming.youtube.com.json" 5. Reopen Profiler, click "Import", import the file from Step 4 AR: Title says "2016" ER: Title should say something more sensible, or at least provide a title with full filename
Has STR: --- → yes
Priority: -- → P3
Whiteboard: [btpp-backlog][good first bug][lang=js]
Hi Greg, Would like to work on this... Could you please explain the file i need to look into and a little more info on why this could be happening. Thanks, Amit.
Hello Amit :) Here's the flow of what's happening: The performance controller reacts to the import button click, and calls the performance actor: https://dxr.mozilla.org/mozilla-central/source/devtools/client/performance/performance-controller.js#325-330 The actor then loads the recording from the file: https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/performance.js#257-274 Here is the actual bug. The filename is turned into the label. As you can see the regex is too greedy finding the last dot: https://dxr.mozilla.org/mozilla-central/source/devtools/client/performance/modules/io.js#107-111
Hi Greg, Thanks for pointing me to the right direction. Please find attached the patch for the changes. Thanks, Amit.
(In reply to Amit Kr. Chandra [:wizard_A] from comment #3) > Created attachment 8744360 [details] [diff] [review] The file is empty.
Hi Greg, Arni, Sorry about that... Please find the attached patch. Thanking You, Amit.
Woops, it looks like we missed a place. It's still clipping the name when you first save the snapshot, but it is fixed when you import. It'd probably be good to hit this one as well. After that it should be good and I'll send it off to the try server to run the tests. https://dxr.mozilla.org/mozilla-central/source/devtools/client/performance/views/recordings.js#214 Thanks for taking this on!
Hi Greg, Hope this resolves the issue.... Thanks..
Comment on attachment 8747430 [details] [diff] [review] Bug-1259227_Change_regex_to_replace_extension.patch Review of attachment 8747430 [details] [diff] [review]: ----------------------------------------------------------------- The code looks good to me, and it has the desired behavior on my local machine. I'm pushing this up to the try servers to run the tests on it. After a clean run I'll mark it to be checked in. Thanks for the work on this!
Attachment #8747430 - Flags: review?(gtatum) → review+
Why is it so necessary to clip something? What if filenames are like "2016-05-04.bak-<timestamp>" ? Where "<timestamp>" is several digits representing timestamp. Debugger, style editor and scratchpad don't clip file extension.
[good first bug] whiteboard -> keyword mass change
I rebased and shortened the commit message a bit.
Attachment #8799970 - Flags: review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/955fd8d87783 Update regex for the imported profile name in the performance tool. r=gregtatum
Hey Amit, thanks for the contribution! Sorry it took so long to get resolved :)
I have successfully reproduce this bug on firefox nightly 48.0a1 (2016-03-23) with windows 7 (32 bit) Mozilla/5.0 (Windows NT 6.1; rv:48.0) Gecko/20100101 Firefox/48.0 I found this fix on latest nightly 52.0a1 (2016-10-16) (32-bit) Mozilla/5.0 (Windows NT 6.1; rv:52.0) Gecko/20100101 Firefox/52.0 Build ID : 20161016030205
You need to log in before you can comment on or make changes to this bug.