Closed Bug 1280091 Opened 8 years ago Closed 8 years ago

Latest Firefox Developer Edition partially breaks ACE editor syntax highlighting

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Version:
49 Branch
Platform:
x86_64
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox48 --- unaffected
firefox49 + fixed
firefox50 + fixed

People

(Reporter: codacodercodacoder, Unassigned)

References

Details

(Keywords: regression, site-compat)

Attachments

(3 files)

ff-deved-ace-fail.jpg
143.83 KB, image/jpeg
Details
ff-deved-ace-fail-2.jpg
309.02 KB, image/jpeg
Details
moz-regress.jpg
235.84 KB, image/jpeg
Details 
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160502172042

Steps to reproduce:

Accepted latest update for Firefox DevEd "49.0a2 (2016-06-14)"


Actual results:

ACE stopped highlighting
OS: Unspecified → Windows 7
Hardware: Unspecified → x86_64
Version: 46 Branch → 49 Branch
Attached image ff-deved-ace-fail.jpg — 
In the image you see (from left to right)

1 - Firefox 46.0.1

2 - Chrome 51.0.2704.84 m

3 - Firefox DevEd 49.0a2 (2016-06-14)
Right now, this is not actively developed code -- has been working "bug free" for many months. When DevEd updated this morning, the problem appeared.

If I reload-reload-reload over and over, I *may* get DevEd to show the highlighting.

Restarting the browser has no effect. Clearing cache has no effect.

I suspect it has something to do with the async nature of ACE's highlighting but I have no evidence to support my "guess".

Notice also, ACE's cold-folding icons don't appear either (evidence it's not my custom DSL syntax highlighting that's the cause and that DevEd is breaking pretty much all of ACE).

For me, this is a pretty catastrophic bug - that's my dev env out the window until this is fixed.
(In reply to Codacoder from comment #2)
> 
> Notice also, ACE's cold-folding icons don't appear 

cold-folding? duh. CODE-FOLDING ;)
Hi Codacoder,

Is there a publicly accessible instance of the issue you're seeing? The syntax highlighting appears to be working for me at https://ace.c9.io/#nav=higlighter -- does it work for you?

Thanks!
Flags: needinfo?(codacodercodacoder)
(it would also be useful to know which version of Ace you're running)
I cannot reproduce the issue with https://ace.c9.io/build/kitchen-sink.html using 49.0a2 (2016-06-14).
@Mike Yes, it does work for me. But my DSL thing is hooked up to ACE as a pseudo-mode - couldn't find a link to the info I followed but it's trivial in the extreme. You manage ace_editor.on("change"...) and run your highlighter using all the usual state stuff and an ACE iterator to step through the file.

No, there's nothing public. It's on my local system and all delivered systems are behind corporate firewalls etc. I am available to do a gotomeeting if you/anyone wants to view my desktop. A few moz devs have done that with me in the past.

I can't find what version is deployed (couldn't find a definitive getVersion or similar on their API page either). I'd say it <12 months old. Nearer 6? 

So, just to update you... I've spent a ton of late-night hours on this and carried on a while this morning too. Next message will have an attachment showing a working "run" from DevEd where ACE has built the ace_marker-layer DIV.

What's so frustrating is, on a failing run, I can spend an age stepping in the debugger and watch the iterator working "perfectly" yet, when I open Inspector and view that div, it's empty.

My next step will be to try to step inside ACE itself - probably a fools errand but I'd just like to know what *it* thinks it's building? And where it thinks it's putting it?

One more point: There are NO console messages (other than my own).

And, in case it needs repeating, this all works "everywhere else" (I have not tested Nightly, don't use it anymore).

Thanks Mike.
Flags: needinfo?(codacodercodacoder)
Attached image ff-deved-ace-fail-2.jpg — 
In a failing run, the highlighted div is EMPTY.
@Mike I guess because it's the highlighter that impacts me (ie my users) I'm tending to focus on that - even in the title of this bug. But it's worth pointing out again, the code-folding is failing too. (But, the line numbering is not failing - different dom element, of course.)

I'm dreading heading into the ACE code... what a time-sink that's gonna be :/
Thanks Codacoder, it would be really useful if you could identify where we regressed using mozregression: http://mozilla.github.io/mozregression/ and paste the result here. 

If we can pin down where it changed (if it's indeed a regression in Gecko), that will save you and us a lot of time.
@Mike - I did the regression-range - I used the gui.

It wasn't clear to me what I was meant to extract from the tool - no "Export Results" command? Or "Send this pile o' **** to Moz dudes" ?

Anyway, my guess is this:

app_name: firefox
build_date: 2016-05-21 04:33:37.152000
build_file: C:\Users\Russ\.mozilla\mozregression\persist\c172b95c757a--mozilla-inbound--firefox-49.0a1.en-US.win64.zip
build_type: inbound
build_url: https://queue.taskcluster.net/v1/task/e1FoA7umTbOnFLoCimR8gA/runs/0/artifacts/public%2Fbuild%2Ffirefox-49.0a1.en-US.win64.zip
changeset: c172b95c757a6242628cd5f0cea2dd0b314b05f7
pushlog_url: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=928fa0c9a879641dcd76b71243da6a6cff70d2d2&tochange=c172b95c757a6242628cd5f0cea2dd0b314b05f7
repo_name: mozilla-inbound
repo_url: https://hg.mozilla.org/integration/mozilla-inbound
task_id: e1FoA7umTbOnFLoCimR8gA

Next comment, I'll paste a screenshot of the gui as it ended the run.

Hope this nails it!
Attached image moz-regress.jpg — 
Mozregression-gui window at end of run.
We can probably re-title this bug...

I disabled my mode completely and ACE still doesn't work (e.g. code-folding is broken). Some parts work, e.g. line numbers, current line highlight to name two. But it seems most other things are MIA.

But I'm not sure what the new title should be: "... partly breaks ACE"?
Blocks: 1273255
Has Regression Range: --- → yes
status-firefox49: --- → affected
Keywords: regressionwindow-wanted
Summary: Latest Firefox Developer Edition breaks ACE editor syntax highlighting → Latest Firefox Developer Edition partially breaks ACE editor syntax highlighting
The bug that regressed this will cause redirects to no longer pre/postfix "www." and ".com" on addresses. What's the http address on which the editor is running, and is it possible some of the resources it needs / requests are being redirected by whatever server is serving them?
Flags: needinfo?(codacodercodacoder)
(In reply to :Gijs Kruitbosch from comment #14)
> The bug that regressed this will cause redirects to no longer pre/postfix
> "www." and ".com" on addresses. What's the http address on which the editor
> is running, and is it possible some of the resources it needs / requests are
> being redirected by whatever server is serving them?

While this would still be interesting to know, I just noted this in the other bug marked as a regression of 1273255:

(In reply to :Gijs Kruitbosch from bug 1281366 comment #4)
> hmm, I don't see the alternate fixing from bug 1273255 being hit at all,
> either in my debugger or when I add printfs. I could be doing something
> wrong, of course - but I think mozregression is misleading us. The two last
> revisions from the output point to:
> 
> https://hg.mozilla.org/integration/mozilla-inbound/
> pushloghtml?fromchange=f66a5b9d&tochange=bc6334ae
> 
> which doesn't just include bug 1273255 but also bug 1267989 and bug 1273661
> and bug 909633.

so it's just as plausible it's one of the other bugs that broke this (though the fact that it works in some pages and not others is certainly odd).
(In reply to :Gijs Kruitbosch from comment #14)
> The bug that regressed this will cause redirects to no longer pre/postfix
> "www." and ".com" on addresses. What's the http address on which the editor
> is running, and is it possible some of the resources it needs / requests are
> being redirected by whatever server is serving them?

Hmm, that's a pretty vague question. Let me try to answer it best I can:

1 - running my dev system, the URL is using http, on customer systems it's most likely https. 

2 - The domain is constant.

3 - As far as I am aware, the installed system never "redirects" (btw, that's a vague use of an overloaded term). There is no url-rewriting in use on the server.

4 - I don't know the exact mechanics of any requests made by ACE.

If you can refine the question, I may be able to give a more definitive answer.

HTH
Flags: needinfo?(codacodercodacoder)
Bug 1281366 ended up being a regression from bug 1267989.

A: http://archive.mozilla.org/pub/firefox/try-builds/ryanvm@gmail.com-1b0207ec7fe0ffd3b24a4d3e03bd9c2cd03011b1/try-win64/firefox-49.0a1.en-US.win64.zip
B: http://archive.mozilla.org/pub/firefox/try-builds/ryanvm@gmail.com-3e509f31053940e4542b184ad8bf87b2a7bc8747/try-win64/firefox-49.0a1.en-US.win64.zip

Codacoder, can you please try out the two builds above? If this is also caused by bug 1267989, the first one should be OK and second not. Note that you might want to use a clean profile for these to avoid any unintended meddling with your regular profiles.
https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles

Thanks for all the help!
Status: UNCONFIRMED → NEW
status-firefox48: --- → unaffected
status-firefox50: --- → affected
tracking-firefox49: --- → ?
tracking-firefox50: --- → ?
Ever confirmed: true
Flags: needinfo?(codacodercodacoder)
Flags: in-testsuite?
Keywords: regression
Bug 1281366 is due to a subtle problem that given just the right timing can cause some scripts on the page to never run when scripts are being dynamically added to the DOM and sync XHR is also in use.

I'll have a patch up in that bug in a bit, and a try run soon after; then it might be possible to just check those builds too.
Depends on: 1281366
(In reply to Ryan VanderMeulen [:RyanVM] from comment #17)
> Bug 1281366 ended up being a regression from bug 1267989.
> 
> A:
> http://archive.mozilla.org/pub/firefox/try-builds/ryanvm@gmail.com-
> 1b0207ec7fe0ffd3b24a4d3e03bd9c2cd03011b1/try-win64/firefox-49.0a1.en-US.
> win64.zip
> B:
> http://archive.mozilla.org/pub/firefox/try-builds/ryanvm@gmail.com-
> 3e509f31053940e4542b184ad8bf87b2a7bc8747/try-win64/firefox-49.0a1.en-US.
> win64.zip
> 
> Codacoder, can you please try out the two builds above? If this is also
> caused by bug 1267989, the first one should be OK and second not. 

CORRECT!

A: PASS

B: FAIL

> Thanks for all the help!

You're welcome. Thanks for the prompt attention, guys!
Flags: needinfo?(codacodercodacoder)
Blocks: 1267989
No longer blocks: 1273255
Component: Untriaged → DOM
Product: Firefox → Core
(In reply to Boris Zbarsky [:bz] (Out June 25-July 6) from comment #19)
> http://archive.mozilla.org/pub/firefox/try-builds/bzbarsky@mozilla.com-
> fd3c7d3baf951d24ad273001b18c8ed3254391b6/try-win64/firefox-50.0a1.en-US.
> win64.zip is a build that has the fix for bug 1281366 in it.

PASS
(In reply to Boris Zbarsky [:bz] (Out June 25-July 6) from comment #18)
> Bug 1281366 is due to a subtle problem that given just the right timing can
> cause some scripts on the page to never run when scripts are being
> dynamically added to the DOM and sync XHR is also in use.

This is a quite complex on a page by page basis) ASP.NET app with sync/async XHR and tons of DOM manip going on. A *lot* happens after original/initial page load. If you haven't described the nature of the cause here, I'd be very surprised.
 
> I'll have a patch up in that bug in a bit, and a try run soon after; then it
> might be possible to just check those builds too.

As I said, PASS. :)
> As I said, PASS. :)

Excellent.  :)
Tracking 49/50+ for this site compat regression
tracking-firefox49: ? → +
tracking-firefox50: ? → +
(In reply to Boris Zbarsky [:bz] (Out June 25-July 6) from comment #23)
> > As I said, PASS. :)
> 
> Excellent.  :)

Hmm, perhaps not, Boris:

https://crash-stats.mozilla.com/report/index/bp-9693bfed-2f66-4d26-b2f6-6c87b2160623
https://crash-stats.mozilla.com/report/index/bp-72d18919-9951-4d3d-b2b4-83eb92160623
https://crash-stats.mozilla.com/report/index/bp-31fd35c2-d510-4079-9246-f78b32160623
https://crash-stats.mozilla.com/report/index/bp-5835a96a-2a17-4499-86d2-8f33f2160623
https://crash-stats.mozilla.com/report/index/bp-4f438c4e-09af-43cd-a9b1-ec6e32160623
https://crash-stats.mozilla.com/report/index/bp-2f12d595-f989-4c93-ab6d-ef6312160411
https://crash-stats.mozilla.com/report/index/bp-4b103cd1-51ce-4853-bbe1-013242160411

These are from the build in comment 19 with multiprocess ENABLED (does not occur with it disabled).

STR:

1 - app has an option to launch a related page (same server, same domain, same everything) in a new window (tab).

2 - perform #1

3 - drag tab to desktop to see it in separate window.

4 - bang. (100% repeatable)

Build A from comment 17 does NOT do this.
Flags: needinfo?(bzbarsky)
@Boris I meant to add in my previous, these are obviously unrelated to THIS bug... but I don't know what to make of them so I'll leave the analysis to you (et al).
Unfortunately, we don't get useful crash stacks from Try builds :(. However, tomorrow's nightly build will contain the fix from bug 1281366, so if you can get the crashes to reproduce in that, we should get useful signatures.
Or if you want, http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-central-win64/1466686706/firefox-50.0a1.en-US.win64.zip should have the fix and generate usable crash stacks if you want to give that a Try.
(In reply to Ryan VanderMeulen [:RyanVM] from comment #28)
> Or if you want,
> http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-central-
> win64/1466686706/firefox-50.0a1.en-US.win64.zip should have the fix and
> generate usable crash stacks if you want to give that a Try.

That doesn't crash. 100% solid.  I ran it with same STR, with multiprocess ENABLED.

File it under WONTFIX-TOO-WEIRD? :)
Heh. Thanks again for all the testing. Bug 1281366 is nominated for uplift to Aurora 49, so the fix should be on all affected releases in the near future.
NP Ryan. When you guys make it easy, it's a pleasure.
So am I correct that this is fixed by the changes from bug 1281366?
Flags: needinfo?(bzbarsky)
You asking me, Boris? Yes. See Comment 22 and 23.
Yes.  Just wanted to confirm before resolving.  ;)
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: