Closed Bug 1260042 Opened 9 years ago Closed 9 years ago

Sources are blank if Auto Prettify Minified Sources enabled

Categories

(DevTools :: Debugger, defect, P1)

Product:
DevTools
Component:
Debugger
Type:
defect

Tracking

(firefox45 wontfix, firefox46+ wontfix, firefox47+ fixed, firefox48+ fixed, firefox49 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 48
Tracking Flags:
Tracking Status
firefox45 --- wontfix
firefox46 + wontfix
firefox47 + fixed
firefox48 + fixed
firefox49 --- fixed

People

(Reporter: arni2033, Assigned: jlong)

References

Details

(Keywords: regression)

Attachments

(3 files)

1260042.patch
2.55 KB, patch
Details | Diff | Splinter Review
1260042-test.patch
3.09 KB, patch
Details | Diff | Splinter Review
1260042-uplift.patch
5.51 KB, patch
ritu
: approval-mozilla-aurora+
ritu
: approval-mozilla-beta+
Details | Diff | Splinter Review
>>> My Info: Win7_64, Nightly 48, 32bit, ID 20160326030430 STR_1: 1. Open attachment 8724506 [details] 2. Open Debugger, enable "Auto Prettify Minified Sources" 3. Close and reopen devtools AR: No code displayed in the code area. ER: The code should be displayed. Notes: 1) This bug is different from "sources are missing". I filed that as bug 1211514, no need to report it 2) In this bug, every time a script is displayed in the list, the code area is empty. That is the bug 3) If you want to workaround bug 1211514, then in Step 3 you should: Switch to Inspector tab -> close and reopen devtools -> Click (ev) button near <html> -> -> click pause button in event listener popup to switch to Debugger. That would reduce the frequency of bug 1211514. STR_2: 1. Open http://www.vesti.ru/doc.html?id=2735707 , wait until the site is fully loaded. 2. Open Debugger, enable "Auto Prettify Minified Sources" 3. Close and reopen devtools 4. If the script selected in the right sidebar is not "26sdov5axQhN", switch to that script This bug is regression from bug 1200798. Regression range: > https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=15522bc2931a52ec435f22e233f747264b90f647&tochange=0e47cb06470145725732f0f362c46048969126d8
Correction: there's a typo in STR_2, Step 4: 4. If the script selected in the LEFT sidebar is not "26sdov5axQhN", switch to that script
James, it seems like this is another regression with auto-prettify. Can you take a look? You did recently fix a related problem in bug 1243561, but this variant still exists in Nightly 48.
Flags: needinfo?(jlong)
Attached patch 1260042.patch — — Splinter Review
Thanks for filing this. Unrelated to the other bug. This happens because prettifying the text fails (because it's not JS, it's HTML) but the editor gets confused because it think it's already displaying the source so it doesn't load in the real source when it comes in. The editor has internal state and only updates the text if it thinks something has changed, and this internal state is wrong. A small fix that I'm going to quickly land.
Flags: needinfo?(jlong)
Assignee: nobody → jlong
Priority: -- → P1
[Tracking Requested - why for this release]: Regression reproduces back to beta. From the regression window, it may be in release as well, be normally we ignore that for DevTools bugs.
status-firefox45: --- → wontfix
status-firefox46: --- → affected
status-firefox47: --- → affected
tracking-firefox46: --- → ?
tracking-firefox47: --- → ?
https://hg.mozilla.org/mozilla-central/rev/6735565943d3
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox48: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
jryans, want to uplift this? We could still take it in beta 7 later this week.
tracking-firefox46: ? → +
tracking-firefox47: ? → +
tracking-firefox48: --- → +
Flags: needinfo?(jryans)
Redirecting to uplift questions assignee.
Flags: needinfo?(jryans) → needinfo?(jlong)
That should have been "Redirect uplift questions to assignee." :)
I can uplift, but I haven't added any tests because I wanted to land P1 fixes quickly and we're working on writing new tests. I wasn't planning on uplifting, but it is a small change I guess.
Flags: needinfo?(jlong)
Comment on attachment 8735449 [details] [diff] [review] 1260042.patch Approval Request Comment [Feature/regressing bug #]: bug 1200798 [User impact if declined]: If the source fails to pretty-print on startup, the source won't be shown in the debugger. [Describe test coverage new/current, TreeHerder]: No new test coverage as we are working on a new test suite and this is a 2 line fix. [Risks and why]: No risk. Only affects debugger and it's a 2 line change. [String/UUID change made/needed]:
Attachment #8735449 - Flags: approval-mozilla-aurora?
(In reply to James Long (:jlongster) from comment #11) > I can uplift, but I haven't added any tests because I wanted to land P1 > fixes quickly and we're working on writing new tests. I wasn't planning on > uplifting, but it is a small change I guess. So, tests are one way we control risk. I think we've settled on some principles about how we control risk in Devtools, and we should deviate from them deliberately. This case seems kind of impulsive. The textual size of the change is irrelevant, of course; what matters is its impact on behavior. The patch does not look entirely trivial to me: we've changed the conditions under which an event is emitted, haven't we?
Comment on attachment 8735449 [details] [diff] [review] 1260042.patch Cancelling uplift as per IRC. Should always have a test if uplifting, I'll add one tomorrow.
Attachment #8735449 - Flags: approval-mozilla-aurora?
When I perform STR_2 on current Nightly (ID 20160330030326), script "26sdov5axQhN" looks minified. So according to the pref "Auto Prettify Minified Sources", it should be pretty printed, but it's not Is that according to the plan?
Flags: needinfo?(jlong)
(In reply to arni2033 from comment #15) > When I perform STR_2 on current Nightly (ID 20160330030326), script > "26sdov5axQhN" looks minified. > So according to the pref "Auto Prettify Minified Sources", it should be > pretty printed, but it's not > Is that according to the plan? Yes. It's an HTML file, and we only pretty-print JS files. We don't know how to pretty-print mixed HTML/JS/CSS files.
Flags: needinfo?(jlong)
(In reply to James Long (:jlongster) from comment #16) > Yes. It's an HTML file, and we only pretty-print JS files. We don't know how > to pretty-print mixed HTML/JS/CSS files. 1) That file is pure JS. In comment 0 I just listed all regressed sources I could find. 2) I saw some mixed files that can be pretty-printed even in Firefox Debugger (e.g. in bug 1258830) 3) (sorry, I literally _just_ checked) It's regression from bug 1200798. I filed bug 1261134. I can verify this particular bug, which is "Sources are blank". Fixed on: Win7_64, Nightly 48, 32bit, ID 20160330030326
Status: RESOLVED → VERIFIED
Hi James, should we consider uplifting this to Beta47? This was marked as P1 regression and therefore caught my attention.
Flags: needinfo?(jlong)
Hi Jen, Joe, could you please help determine if this needs to be uplifted to Fx47? If yes, please nominate for uplift as requested by me in comment 19. Thanks!
Flags: needinfo?(jwalker)
Flags: needinfo?(jfong)
I'm sorry I've been slow on this -- I've been focusing on a different project for a little while now. I need to write a test for this and then I think we should uplift this. I will do that tomorrow.
Flags: needinfo?(jlong)
See comment 21
Flags: needinfo?(jwalker)
Re-opening to land a test for this. I will generate a patch with the test to be uplifted to beta.
Status: VERIFIED → REOPENED
Flags: needinfo?(jfong)
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/a67f6710c37c
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Comment on attachment 8752991 [details] [diff] [review] 1260042-uplift.patch Approval Request Comment [Feature/regressing bug #]: NA [User impact if declined]: Auto-pretty-printing may not work in debugger [Describe test coverage new/current, TreeHerder]: New test added, has been on nightly for a while [Risks and why]: Very little risk, only a few lines changed and a new test has been added [String/UUID change made/needed]:
Attachment #8752991 - Flags: approval-mozilla-beta?
Attachment #8752991 - Flags: approval-mozilla-aurora?
Comment on attachment 8752991 [details] [diff] [review] 1260042-uplift.patch Fix was verified on Aurora48, Nightly49. Not sure if this needs uplift to 48. Beta47+
Attachment #8752991 - Flags: approval-mozilla-beta?
Attachment #8752991 - Flags: approval-mozilla-beta+
Attachment #8752991 - Flags: approval-mozilla-aurora?
Attachment #8752991 - Flags: approval-mozilla-aurora+
I'm hitting conflicts uplifting this to aurora: hg import -e https://bugzilla.mozilla.org/attachment.cgi?id=8752991 applying https://bugzilla.mozilla.org/attachment.cgi?id=8752991 patching file devtools/client/debugger/content/reducers/sources.js Hunk #1 FAILED at 61 1 out of 1 hunks FAILED -- saving rejects to file devtools/client/debugger/content/reducers/sources.js.rej patching file devtools/client/shared/redux/non-react-subscriber.js Hunk #1 FAILED at 83 1 out of 1 hunks FAILED -- saving rejects to file devtools/client/shared/redux/non-react-subscriber.js.rej abort: patch failed to apply
Flags: needinfo?(jlong)
(In reply to Wes Kocher (:KWierso) from comment #31) > I'm hitting conflicts uplifting this to aurora: > > hg import -e https://bugzilla.mozilla.org/attachment.cgi?id=8752991 > applying https://bugzilla.mozilla.org/attachment.cgi?id=8752991 > patching file devtools/client/debugger/content/reducers/sources.js > Hunk #1 FAILED at 61 > 1 out of 1 hunks FAILED -- saving rejects to file > devtools/client/debugger/content/reducers/sources.js.rej > patching file devtools/client/shared/redux/non-react-subscriber.js > Hunk #1 FAILED at 83 > 1 out of 1 hunks FAILED -- saving rejects to file > devtools/client/shared/redux/non-react-subscriber.js.rej > abort: patch failed to apply I didn't realize this, but it's actually already been uplifted to aurora. This needs to be uplifted to beta only.
Flags: needinfo?(jlong)
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: