Bug 235607 – No breakpoints hit on github.com, and some are invalid

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Dean Jackson]

Created attachment 449962 [details]
Screenshot

1. Go to github.com when not logged in (so you get the globe animation, not your dashboard).

2. Open the inspector, search for "getContext(" and put a breakpoint on the line with "Y = e.getContext(F, H);"

3. Put another breakpoint just below that, on the line with "const x = {". Notice that it is not resolved/enabled.

4. Reload the page. None of the breakpoints are hit.

[Radar WebKit Bug Importer]

<rdar://problem/88042400>

[Patrick Angle]

Created attachment 449986 [details]
Patch v1.0 - Needs tests

[Patrick Angle]

Created attachment 450500 [details]
Patch v1.1 - Added tests, fixed cases where string token start offset was incorrect.

[Patrick Angle]

Created attachment 450502 [details]
Patch v1.2 - Pass parser-xml-close-comment.html again

[Yusuke Suzuki]

Comment on attachment 450502 [details]
Patch v1.2 - Pass parser-xml-close-comment.html again

View in context: https://bugs.webkit.org/attachment.cgi?id=450502&action=review

r=me

> Source/JavaScriptCore/parser/Lexer.cpp:2493
>          }

I know it is pre-existing, but this }'s place is wrong, so can you unindent one level?

[Joseph Pecoraro]

Comment on attachment 450502 [details]
Patch v1.2 - Pass parser-xml-close-comment.html again

Nice!

[Patrick Angle]

Created attachment 450645 [details]
Patch v1.3 - Review nit

[EWS]

Committed r288996 (246715@main): <https://commits.webkit.org/246715@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 450645 [details].

[WebKit Commit Bot]

Re-opened since this is blocked by bug 236141

[Patrick Angle]

Created attachment 455382 [details]
Patch v1.4 - Fix handling multi-line comment and string error cases

[Patrick Angle]

Comment on attachment 455382 [details]
Patch v1.4 - Fix handling multi-line comment and string error cases

Oops - looks like this patch doesn't actually include the new test file 🤦🏻‍♂️

[Patrick Angle]

Created attachment 455396 [details]
Patch v1.4a - Fix handling multi-line comment and string error cases

[Yusuke Suzuki]

Comment on attachment 455396 [details]
Patch v1.4a - Fix handling multi-line comment and string error cases

View in context: https://bugs.webkit.org/attachment.cgi?id=455396&action=review

r=me

> Source/JavaScriptCore/ChangeLog:7
> +

Can you also add JSTests/stress test which caused the crash for the previous patch? (To ensure that this new one is working).

[Patrick Angle]

Created attachment 455422 [details]
Patch v1.5 - Add JSTests cases

[EWS]

Committed r291746 (248778@main): <https://commits.webkit.org/248778@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 455422 [details].

[Yusuke Suzuki]

*** Bug 221548 has been marked as a duplicate of this bug. ***

[Yusuke Suzuki]

*** Bug 213873 has been marked as a duplicate of this bug. ***

[Yusuke Suzuki]

*** Bug 239491 has been marked as a duplicate of this bug. ***

[Patrick Angle]

*** Bug 244814 has been marked as a duplicate of this bug. ***

[Alexey]

Has it landed on any stable version yet?