Closed Bug 1214663 Opened 9 years ago Closed 9 years ago

Can't view source of many XBL bindings in debugger (and therefore can't debug those files)

Categories

(DevTools :: Debugger, defect)

Product:
DevTools
Component:
Debugger
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

(firefox44 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 44
Tracking Flags:
Tracking Status
firefox44 --- fixed

People

(Reporter: Gijs, Assigned: bgrins)

References

(
URL
)

Details

(Keywords: regression)

Attachments

(1 file)

MozReview Request: Bug 1214663 - Make CodeMirror not break when encountering a <constructor> tag;r=Gijs
40 bytes, text/x-review-board-request
Gijs
: review+
Details 
The regression window helpfully includes bug 1132557, which seems like a likely culprit.

Gabriel, can you look into this?
Blocks: 1132557
Flags: needinfo?(gabriel.luong)
Keywords: regression
(In reply to :Gijs Kruitbosch from comment #2)
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=6256ec9113c115141aab089c45ee69438884b680&tochange=ccee
> 6614fd9d18a31f263fbcfe9676b224d851aa for the m-c window, fwiw

Confirmed that this regressed as part of:

https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=21015383ac41061eab25d01b546cd2713ff17b36&tochange=2bfe5435c1d1e2784af3b225d629a210023daecd
... though actually, that regression range is for the toolbar.xml file, but urlbarbindings.xml kept working for longer and broke more recently... sigh. Let me try to find a second regression range.
(In reply to :Gijs Kruitbosch from comment #4)
> ... though actually, that regression range is for the toolbar.xml file, but
> urlbarbindings.xml kept working for longer and broke more recently... sigh.
> Let me try to find a second regression range.

... annnd it looks like that's specific to my profile. So let's focus on the clean-profile regression and I'll try to work out how my profile is different in a separate bug. Updated STR:

1. open browser toolbox
2. switch to debugger
3. open urlbarbindings.xml
4. open content/customizableui/toolbar.xml

and a blank page (but with scrollbars?) greets you.

I still have no idea why some files work and others do not.
In fact, I see urlbarbindings.xml blank out when scrolling down in the file (and it seems to randomly come back into view after scrolling left and right a bit)..
Ah, in the browser console for the BT window I'm seeing:

TypeError: spec[0] is undefined

findMatchingMode()
 htmlmixed.js:72
html()
 htmlmixed.js:98
.token()
 htmlmixed.js:131
readToken()
 codemirror.js:6719
runMode()
 codemirror.js:6762
highlightLine()
 codemirror.js:6794
getLineStyles()
 codemirror.js:6830
buildLineContent()
 codemirror.js:6891
getLineContent()
 codemirror.js:923
buildLineElement()
 codemirror.js:1006
patchDisplay()
 codemirror.js:854
updateDisplayIfNeeded()
 codemirror.js:707
updateDisplaySimple()
 codemirror.js:761
setScrollTop()
 codemirror.js:3914
registerEventHandlers/<()
 codemirror.js:3446
 htmlmixed.js:72:11
Ha, well it's choking whenever it hits a <constructor> tag.

The reason is that this is undefined because tagInfo is a function and it's expecting an array here: https://github.com/codemirror/CodeMirror/blob/0ca9fc878df2ecc2f9d2a279de9d5c3786f9a2f9/mode/htmlmixed/htmlmixed.js#L72.

The reason it's getting passed to that function is that it's assigning `tags["constructor"]` and then just null checking (not checking to make sure it's the right kind of object): https://github.com/codemirror/CodeMirror/blob/0ca9fc878df2ecc2f9d2a279de9d5c3786f9a2f9/mode/htmlmixed/htmlmixed.js#L93
Bug 1214663 - Make CodeMirror not break when encountering a <constructor> tag;r=Gijs
Attachment #8673723 - Flags: review?(gijskruitbosch+bugs)
(In reply to Brian Grinstead [:bgrins] from comment #9)
> Created attachment 8673723 [details]
> MozReview Request: Bug 1214663 - Make CodeMirror not break when encountering
> a <constructor> tag;r=Gijs
> 
> Bug 1214663 - Make CodeMirror not break when encountering a <constructor>
> tag;r=Gijs

Can you confirm this fixes the problem?  I will open a PR upstream for CodeMirror if so
Comment on attachment 8673723 [details]
MozReview Request: Bug 1214663 - Make CodeMirror not break when encountering a <constructor> tag;r=Gijs

https://reviewboard.mozilla.org/r/21955/#review19735

Yes, much better, thank you!

::: devtools/client/sourceeditor/codemirror/mode/htmlmixed.js:94
(Diff revision 1)
> +      var isTagInfoArray = Object.prototype.toString.call(tagInfo) === "[object Array]";

Any particular reason not to use Array.isArray when available? Seems CM supports IE8 so we can't rely only on that, but this seems particularly yucky...
Attachment #8673723 - Flags: review?(gijskruitbosch+bugs) → review+
Assignee: nobody → bgrinstead
URL: https://github.com/codemirror/CodeMir...
Status: NEW → ASSIGNED
(In reply to :Gijs Kruitbosch from comment #11)
> Comment on attachment 8673723 [details]
> MozReview Request: Bug 1214663 - Make CodeMirror not break when encountering
> a <constructor> tag;r=Gijs
> 
> https://reviewboard.mozilla.org/r/21955/#review19735
> 
> Yes, much better, thank you!
> 
> ::: devtools/client/sourceeditor/codemirror/mode/htmlmixed.js:94
> (Diff revision 1)
> > +      var isTagInfoArray = Object.prototype.toString.call(tagInfo) === "[object Array]";
> 
> Any particular reason not to use Array.isArray when available? Seems CM
> supports IE8 so we can't rely only on that, but this seems particularly
> yucky...

Yeah, just wanting to make sure we won't need to make customizations from the upstream repo.  I'll make a note in the PR about Array.isArray though
Comment on attachment 8673723 [details]
MozReview Request: Bug 1214663 - Make CodeMirror not break when encountering a <constructor> tag;r=Gijs

Bug 1214663 - Make CodeMirror not break when encountering a <constructor> tag;r=Gijs
Flags: needinfo?(gabriel.luong)
https://hg.mozilla.org/mozilla-central/rev/46457432906d
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox44: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
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: