1282332 - http://plus.timescar.jp/view/station/detail.jsp?scd=V086 fails to load content

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brian Birtles (:birtles, Feb 26~ parental leave)]

See: https://github.com/webcompat/web-bugs/issues/2797

Loading the bug URL in Firefox 49 or later shows a page that gets stuck loading content. It says, ただいま地図を読み込み中です... ("Currently downloading map...").

In the console there is an unhandled exception which seems to be related to the AJAX request (based on the stack): "TypeError: str.match(...) is null"

mozregression points to bug 1266255 as the cause.

Loading the same URL in Chrome/Edge/Firefox 48 or less, displays the content as expected.

Comment 1

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Morgan, do you know what is happening here?

Comment 2

[Morgan Phillips [:mrrrgn]]

I see the error, we're incorrectly deriving the function's name from it's display atom: ".Deferred/</i[o[0]]()" to "0]" when it should be "".

Comment 3

[Morgan Phillips [:mrrrgn]]

Attachment: displayname.diff

This is so ... yuck :( I'm going to see about ripping this entire "display name parsing" mechanism when I work on computed property names.

Comment 4

[Jason Orendorff [:jorendorff]]

Comment on attachment 8765309 [details] [diff] [review]
displayname.diff

Review of attachment 8765309 [details] [diff] [review]:
-----------------------------------------------------------------

Ugh. Maybe it is time to throw away displayName.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b85f918f0db
Refuse to parse display names with unquoted, non-numeric, property names; r=jorendorff

Comment 6

[Morgan Phillips [:mrrrgn]]

(In reply to Jason Orendorff [:jorendorff] from comment #4)
> Comment on attachment 8765309 [details] [diff] [review]
> displayname.diff
> 
> Review of attachment 8765309 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Ugh. Maybe it is time to throw away displayName.

Maybe I can do all of this with a new opcode: JSOP_SETFNAME (or something along those lines). If not, yes, I'll open a bug to get a dialogue started with devtools. It's a real thorn! <sub>ps, thx for the quick review. :]</sub>

Comment 7

[Morgan Phillips [:mrrrgn]]

s/I/we/g

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9b85f918f0db

Comment 9

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Thanks for the quick fix! We'll want to uplift this to Aurora once it's baked on trunk.

Comment 10

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Hi Morgan, thanks for fixing this so quickly. Would you mind requesting aurora approval? Thanks!

Comment 11

[Morgan Phillips [:mrrrgn]]

(In reply to Brian Birtles (:birtles) from comment #10)
> Hi Morgan, thanks for fixing this so quickly. Would you mind requesting
> aurora approval? Thanks!

Derp! Thanks for the reminder, and for reporting this! :)

Comment 12

[Morgan Phillips [:mrrrgn]]

Comment on attachment 8765309 [details] [diff] [review]
displayname.diff

Approval Request Comment
[Feature/regressing bug #]: 1282332
[User impact if declined]: Broken web pages.
[Describe test coverage new/current, TreeHerder]: Tests are included with the patch.
[Risks and why]: Risks are minimal. The changes are isolated.
[String/UUID change made/needed]:

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8765309 [details] [diff] [review]
displayname.diff

Fix for regression from 49, includes tests, let's take it!

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b27163b500de