From 47c1ea38e952cf3103e6e4c35c67ea76bf23834a Mon Sep 17 00:00:00 2001 From: Alexandre Poirot Date: Thu, 3 Apr 2025 11:58:29 +0000 Subject: [PATCH] Bug 1953394 - [devtools] Scroll to center new pause location with column being out of the viewport. r=devtools-reviewers,bomsy Before this patch, it would only make the column visible on the edge of the viewport, so that we could only see the breakpoint marker, and/or only the first character after the pause location. With this patch, if the paused location is visible, nothing happens. But if the paused location is outside of the viewport, the paused location is centered horizontally. Differential Revision: https://phabricator.services.mozilla.com/D241173 --- .../debugger/test/mochitest/browser_aj.toml | 2 + .../browser_dbg-editor-horizontal-scroll.js | 180 ++++++++++++++++++ .../mochitest/browser_dbg-editor-scroll.js | 45 ++++- .../debugger/test/mochitest/examples/Ahem.ttf | Bin 0 -> 12480 bytes .../mochitest/examples/doc-editor-scroll.html | 1 + .../mochitest/examples/horizontal-scroll.js | 2 + devtools/client/shared/sourceeditor/editor.js | 10 +- 7 files changed, 230 insertions(+), 10 deletions(-) create mode 100644 devtools/client/debugger/test/mochitest/browser_dbg-editor-horizontal-scroll.js create mode 100644 devtools/client/debugger/test/mochitest/examples/Ahem.ttf create mode 100644 devtools/client/debugger/test/mochitest/examples/horizontal-scroll.js diff --git a/devtools/client/debugger/test/mochitest/browser_aj.toml b/devtools/client/debugger/test/mochitest/browser_aj.toml index 7484ca9edfd8..8189422ccc08 100644 --- a/devtools/client/debugger/test/mochitest/browser_aj.toml +++ b/devtools/client/debugger/test/mochitest/browser_aj.toml @@ -185,6 +185,8 @@ fail-if = ["a11y_checks"] # Bug 1849028 clicked element may not be focusable and ["browser_dbg-editor-scroll.js"] skip-if = ["cm5"] +["browser_dbg-editor-horizontal-scroll.js"] + ["browser_dbg-editor-select.js"] ["browser_dbg-ember-original-variable-mapping-notifications.js"] diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-editor-horizontal-scroll.js b/devtools/client/debugger/test/mochitest/browser_dbg-editor-horizontal-scroll.js new file mode 100644 index 000000000000..4f4ad7a5e3e0 --- /dev/null +++ b/devtools/client/debugger/test/mochitest/browser_dbg-editor-horizontal-scroll.js @@ -0,0 +1,180 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at . */ + +// Tests that the editor scrolls correctly when pausing on location that +// requires horizontal scrolling. + +"use strict"; + +add_task(async function testHorizontalScrolling() { + if (!isCm6Enabled) { + ok(true, "This test is disabled on CM5"); + return; + } + + // Ensure having the default fixed height, as it can impact the number of displayed lines + await pushPref("devtools.toolbox.footer.height", 250); + + // Also set a precise size for side panels, as it can impact the number of displayed columns + await pushPref("devtools.debugger.start-panel-size", 300); + await pushPref("devtools.debugger.end-panel-size", 300); + + // Strengthen the test by ensuring we always use the same Firefox window size. + // Note that the inner size is the important one as that's the final space available for DevTools. + // The outer size will be different based on OS/Environment. + const expectedWidth = 1280; + const expectedHeight = 1040; + if ( + window.innerWidth != expectedWidth || + window.innerHeight != expectedHeight + ) { + info("Resize the top level window to match the expected size"); + const onResize = once(window, "resize"); + const deltaW = window.outerWidth - window.innerWidth; + const deltaH = window.outerHeight - window.innerHeight; + const originalWidth = window.outerWidth; + const originalHeight = window.outerHeight; + window.resizeTo(expectedWidth + deltaW, expectedHeight + deltaH); + await onResize; + registerCleanupFunction(() => { + window.resizeTo(originalWidth, originalHeight); + }); + } + is(window.innerWidth, expectedWidth); + + const dbg = await initDebugger( + "doc-editor-scroll.html", + "scroll.js", + "long.js" + ); + + await selectSource(dbg, "horizontal-scroll.js"); + const editor = getCMEditor(dbg); + + const global = editor.codeMirror.contentDOM.ownerGlobal; + const font = new global.FontFace( + "Ahem", + "url(chrome://mochitests/content/browser/devtools/client/debugger/test/mochitest/examples/Ahem.ttf)" + ); + const loadedFont = await font.load(); + global.document.fonts.add(loadedFont); + + is(global.devicePixelRatio, 1); + is(global.browsingContext.top.window.devicePixelRatio, 1); + global.browsingContext.top.overrideDPPX = 1; + is(global.browsingContext.fullZoom, 1); + is(global.browsingContext.textZoom, 1); + + // /!\ Change the Codemirror font to use a fixed font across all OSes + // and always have the same number of characters displayed. + // Note that this devtools mono makes the "o" characters almost invisible. + editor.codeMirror.contentDOM.style.fontFamily = "Ahem"; + editor.codeMirror.contentDOM.style.fontSize = "10px"; + editor.codeMirror.contentDOM.style.lineHeight = "15px"; + editor.codeMirror.contentDOM.style.fontWeight = "normal"; + editor.codeMirror.contentDOM.style.fontStyle = "normal"; + editor.codeMirror.contentDOM.style.fontStretch = "normal"; + is(global.getComputedStyle(editor.codeMirror.contentDOM).fontFamily, "Ahem"); + + await wait(1000); + + is( + Math.round(editor.codeMirror.dom.getBoundingClientRect().width), + 679, + "Sanity check to ensure we have a fixed editor width, so that we have the expected displayed columns" + ); + + // All the following methods lookup for first/last visible position in the current viewport. + // Also note that the element at the returned position may only be partially visible. + function getFirstVisibleColumn() { + const { x, y } = editor.codeMirror.dom.getBoundingClientRect(); + const gutterWidth = + editor.codeMirror.dom.querySelector(".cm-gutters").clientWidth; + // This is hardcoded to match the second line, which is around 20px from the top. + // Also append the gutter width as it would pick hidden columns displayed behind it + const pos = editor.codeMirror.posAtCoords({ + x: x + gutterWidth + 2, + y: y + 20, + }); + // /!\ the column is 0-based while lines are 1-based + return pos - editor.codeMirror.state.doc.lineAt(pos).from; + } + function getLastVisibleColumn() { + const { x, y, width } = editor.codeMirror.dom.getBoundingClientRect(); + // This is hardcoded to match the second line, which is around 20px from the top + const pos = editor.codeMirror.posAtCoords({ x: x + width, y: y + 20 }); + // /!\ the column is 0-based while lines are 1-based + return pos - editor.codeMirror.state.doc.lineAt(pos).from; + } + + info("Pause in middle of the screen, we should not scroll on pause"); + await addBreakpoint(dbg, "horizontal-scroll.js", 2, 25); + invokeInTab("horizontal"); + await waitForPaused(dbg); + + const lastColumn = getLastVisibleColumn(); + is(lastColumn, 55); + ok( + isScrolledPositionVisible(dbg, 2, 1), + "The 2nd line, first column is visible" + ); + ok( + !isScrolledPositionVisible(dbg, 2, lastColumn), + "The 2nd line, last column is partially visible and considered hidden" + ); + ok( + isScrolledPositionVisible(dbg, 2, lastColumn - 1), + "The column before the last column is visible" + ); + + info("Step to the last visible column, the editor shouldn't scroll"); + // This breakpoint location is on the last visible column and would not cause a scroll. + await addBreakpoint(dbg, "horizontal-scroll.js", 2, lastColumn); + await resume(dbg); + await waitForPaused(dbg); + + is(getLastVisibleColumn(), lastColumn, "We did not scroll horizontaly"); + ok( + !isScrolledPositionVisible(dbg, 2, lastColumn), + "The last column is still considered hidden" + ); + ok( + isScrolledPositionVisible(dbg, 2, lastColumn - 1), + "The column before the last colunm is still visible" + ); + + info( + "Step to the next column, and the editor should scroll it into the center" + ); + + info("Step into the next breakable column, the editor should now scroll"); + // Set a breakpoint to the next breakable position (there is one every two columns, and lastColumn was breakable) + await addBreakpoint(dbg, "horizontal-scroll.js", 2, lastColumn + 2); + await resume(dbg); + await waitForPaused(dbg); + + const lastColumn2 = getLastVisibleColumn(); + + is(lastColumn2, 74); + ok( + isScrolledPositionVisible(dbg, 2, lastColumn2), + "The new last column is visible" + ); + ok( + !isScrolledPositionVisible(dbg, 2, lastColumn2 + 1), + "The column after the last is hidden" + ); + const firstColumn = getFirstVisibleColumn(); + is(firstColumn, 30); + ok( + !isScrolledPositionVisible(dbg, 2, firstColumn), + "The new first column is partially visible and considered hidden" + ); + ok( + isScrolledPositionVisible(dbg, 2, firstColumn + 1), + "The column after the first visible is visible" + ); + + await resume(dbg); +}); diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-editor-scroll.js b/devtools/client/debugger/test/mochitest/browser_dbg-editor-scroll.js index 7348e10cb5c4..ac181f98f388 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-editor-scroll.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-editor-scroll.js @@ -89,6 +89,33 @@ add_task(async function testIsPositionVisible() { // Ensure having the default fixed height, as it can impact the number of displayed lines await pushPref("devtools.toolbox.footer.height", 250); + // Also set a precise size for side panels, as it can impact the number of displayed columns + await pushPref("devtools.debugger.start-panel-size", 300); + await pushPref("devtools.debugger.end-panel-size", 300); + + // Strengthen the test by ensuring we always use the same Firefox window size. + // Note that the inner size is the important one as that's the final space available for DevTools. + // The outer size will be different based on OS/Environment. + const expectedWidth = 1280; + const expectedHeight = 1040; + if ( + window.innerWidth != expectedWidth || + window.innerHeight != expectedHeight + ) { + info("Resize the top level window to match the expected size"); + const onResize = once(window, "resize"); + const deltaW = window.outerWidth - window.innerWidth; + const deltaH = window.outerHeight - window.innerHeight; + const originalWidth = window.outerWidth; + const originalHeight = window.outerHeight; + window.resizeTo(expectedWidth + deltaW, expectedHeight + deltaH); + await onResize; + registerCleanupFunction(() => { + window.resizeTo(originalWidth, originalHeight); + }); + } + is(window.innerWidth, expectedWidth); + const dbg = await initDebugger( "doc-editor-scroll.html", "scroll.js", @@ -98,18 +125,20 @@ add_task(async function testIsPositionVisible() { await selectSource(dbg, "scroll.js"); const editor = getCMEditor(dbg); - function getFirstLine() { + // All the following methods lookup for first/last visible position in the current viewport. + // Also note that the element at the returned position may only be partially visible. + function getFirstVisibleLine() { const { x, y } = editor.codeMirror.dom.getBoundingClientRect(); // Add a pixel as we may be on the edge of the previous line which is hidden const pos = editor.codeMirror.posAtCoords({ x, y: y + 1 }); return editor.codeMirror.state.doc.lineAt(pos).number; } - function getLastLine() { + function getLastVisibleLine() { const { x, y, height } = editor.codeMirror.dom.getBoundingClientRect(); const pos = editor.codeMirror.posAtCoords({ x, y: y + height }); return editor.codeMirror.state.doc.lineAt(pos).number; } - const lastLine = getLastLine(); + const lastLine = getLastVisibleLine(); is( lastLine, @@ -145,13 +174,13 @@ add_task(async function testIsPositionVisible() { await resume(dbg); info( - "Set a breakpoint on the last partially visibible line, it should scroll that line in the middle of the viewport" + "Set a breakpoint on the last partially visible line, it should scroll that line in the middle of the viewport" ); await addBreakpoint(dbg, "scroll.js", lastLine); invokeInTab("line" + lastLine); await waitForPaused(dbg); - const newLastLine = getLastLine(); + const newLastLine = getLastVisibleLine(); is(newLastLine, 16, "The new last line is the 16th"); ok( !isScrolledPositionVisible(dbg, newLastLine), @@ -161,7 +190,7 @@ add_task(async function testIsPositionVisible() { isScrolledPositionVisible(dbg, newLastLine - 1), "The line before is reported as visible" ); - const firstLine = getFirstLine(); + const firstLine = getFirstVisibleLine(); is(firstLine, 6); ok( isScrolledPositionVisible(dbg, firstLine), @@ -181,7 +210,7 @@ add_task(async function testIsPositionVisible() { invokeInTab("line50"); await waitForPaused(dbg); - const newLastLine2 = getLastLine(); + const newLastLine2 = getLastVisibleLine(); is(newLastLine2, 55); ok( !isScrolledPositionVisible(dbg, newLastLine2), @@ -191,7 +220,7 @@ add_task(async function testIsPositionVisible() { isScrolledPositionVisible(dbg, newLastLine2 - 1), "The line before is visible" ); - const firstLine2 = getFirstLine(); + const firstLine2 = getFirstVisibleLine(); is(firstLine2, 45); ok( isScrolledPositionVisible(dbg, firstLine2), diff --git a/devtools/client/debugger/test/mochitest/examples/Ahem.ttf b/devtools/client/debugger/test/mochitest/examples/Ahem.ttf new file mode 100644 index 0000000000000000000000000000000000000000..ac81cb03165ab831a36abb59145ff7a2f5675fb9 GIT binary patch literal 12480 zcmeHNZFm&b6@KSqXFq`?V#J8_W{Z(327;C%rN~F5ND+}BqNU0vJIMyKyK#48f&xWr z)kgU!T8&C6B7%Y<3R(n85fK4VsitTVmo!>UZB&Y2eGpjRJ3A2++dqB$VLP+W?3{b< zy=Tt7=bd}c?z~U{_%Rb2`d(7jr(eU^QL_ML0JW1VqM_O@yYfc>t{T8dRE3hYI0asy z?@HF^RMkwayl7QwHx0A^1>uMp3h!DpraSA_^Xwm?!tK-ZDIeh3GZIZTJR>lj_L~5$ zxh7r_5_M+=fud6C+M}U{T6`+)E8W;$#6nSXM%O#m0KNM1{*l^vGBs=G>5V`!`>Wj` zeOE9n@03fcuI@7EAiP=|HCPqD`U-j7c+T>RfXo1`O%p6?P^Fd!`)MLGfZtYoeoITo z9|tYXL3r#wU#;iuwKiogTol%;^ayjZSLJ#1Q#5L!a~$%R2|S(yG14mV>+*Al14AJ8 zskRRSTLqNdA*|Dc<*Bed<2+?OlwLZgGY1X^zTFs*k8}3aN82k-cWSt{s6{YTpM2g~ z`084ta52dq9ap<}!X?{NgU)ryfSaqAT!VaE-vj7`&gg<1CKG*VYI28pr z4c&1%df+<{C`1t7MNgc8BAkg{_#V!}+31aPP>eq4ixTw1xi}BsM}PbP18_bD;)fW7 zA7L;q;JP*x!!R7BxDX@oV~oT_7=@o;G%m&%T!OK<6ys2a@wf~Va5*O83S5bwViJCa zt8g_Y)87!vQGqZ_RH6zIRAUORK@FmaA&y#Hiv*HLp$^xf9u1g^>v037abKB%pL4l= z6F1@}`~o-QE&LL<;8xs*xA7}9;(gqXJJ5z%_y^iC8*}gu-o@Y0jK5pKVui(z+doNEWq#ZAr@jMTJZo{@K-A$d$0(v zV>kBVK`h2YSc0W^7>{5Xmg7-8#x^VPI9B2bJc(6Uji>N5evdVH25Yg7JEu-GfU`A( zQS8hp%s@sdQOcACWuE$ix=npcyGwgSTd!>tIijm55Isat6p6lKpeQX278V87pb_j6 z%nKF-dj!u8jtou?n!){NY~QD5GPElg(}lD*&a$_K_TD(o-btdH=q`k1@BA;>%MG6T zg}tz451CBn?My3o*c&s`G7~cw07u$+4nA_$;n$9I-J5CI1hlMgS=X|r}@&0b7ZTImF2#&i@u-pb6Wt4KES}_c3-4GU^hK zw-b>|iOI!`=5jll8;H;qRz&9$sm~KxPgrrS!bWT&?y8C1Da7qHMD9Fdx0dLY*i8|= z61#sSdZ!Y<*Au%p5J`73($g&jKa1yxs#U~778&b^lC>7zUMF5R)7SSLlwIg)X8d0ou)RbP3jVLwYo{&sixI-^^oS#^0l5? zKW&IMMw_Hf(HgW{_#S(awo==m?a+49(@%6=&(nootPj#h=@a#^p3rCNv-SDgDS1Ds_!>O?Jgx(_D?NCf5?zYS$*$PFLF1?mA?6jC`Y~(a#uS zj4>t|Q;Y`V7UM2sk+IU)VC*n<8*RoXZrz>d7VcvAAonQuM0ce-<-W;1*S)~~sC%t@ zt9zIG9ru0@Je@t=J!g3a@H4_Q-c#wVjOZ}{Hz9q_CE9DfggZ~s95NdE+X*q`vv^w0Ls_b>CW@o(`r`&<3{{6_ul3(%P!uhX4o>vqA#{(jzxde zmN^!Etu1pb`fgk1SoBs~=2-MkZ8--Xm;4`7+RsI2m&ca#sLv-OCT{Uq1i1wfvJ4VD zE65HoEq;rzG$sT!5$XibD8G~B5Y*Xqy)A76dzoYxq*y8_g)Iq%2(Eaogq z8&R^AOtO&Vm}G<|Sgob!S+7gzX$h@$wC=cmeLK7MV)9r;MI@96Riw;BQiKwwNJUIx zMn&@4I+l71v!TMQO^KolhgQ_3Oc9EOMX)y5ThvFYDB4 zsAZ)*lr(FqV`j0qENNEO)riV?LZr-OsybFBD&kSHXsc7z@t8}O@731UKuow}FBw}VF+mntBFDnyep=ezqRDEVrlqcf#N!Hpni-$(c=s2^gt|pY2 zz~M;KCB-E|s^xi{u0v(FmPO|&_Xp^ zka_nJ_Tit{kB`anKZsAPxJh>7I&$S6Ca>*&i|4r6VmdxgM&2f}@0OEmw}A|}^<>R8 zk!g1?xpwo&ubgACFPD;w_bOR*&yq>6D!M-AveBc}k>j<}3rEwsX0h^fppeg~eGU!H Bc@h8s literal 0 HcmV?d00001 diff --git a/devtools/client/debugger/test/mochitest/examples/doc-editor-scroll.html b/devtools/client/debugger/test/mochitest/examples/doc-editor-scroll.html index 3ddcbd65ad2f..b5125262475f 100644 --- a/devtools/client/debugger/test/mochitest/examples/doc-editor-scroll.html +++ b/devtools/client/debugger/test/mochitest/examples/doc-editor-scroll.html @@ -10,6 +10,7 @@ + diff --git a/devtools/client/debugger/test/mochitest/examples/horizontal-scroll.js b/devtools/client/debugger/test/mochitest/examples/horizontal-scroll.js new file mode 100644 index 000000000000..324ee3c6e38f --- /dev/null +++ b/devtools/client/debugger/test/mochitest/examples/horizontal-scroll.js @@ -0,0 +1,2 @@ +let a="a",b="b",c="c",d="d",e="e",f="f",g="g",h="h",i="i",j="j",k="k",l="l",m="m",n="n",o="o",p="p",q="q",r="r",s="s",t="t",u="u",v="v",x="x",y="y",z="z"; +function horizontal() { console.log("horizontal");a;b;c;d;e;f;g;h;i;j;k;l;m;n;o;p;q;r;s;t;u;v;x;y;z;a;b;c;d;e;f;g;h;i;j;k;l;m;n;o;p;q;r;s;t;u;v;x;y;z;a;b;c;d;e;f;g;h;i;j;k;l;m;n;o;p;q;r;s;t;u;v;x;y;z;a;b;c;d;e;f;g;h;i;j;k;l;m;n;o;p;q;r;s;t;u;v;x;y;z } diff --git a/devtools/client/shared/sourceeditor/editor.js b/devtools/client/shared/sourceeditor/editor.js index 6a1cb41e0c96..b7525235dacc 100644 --- a/devtools/client/shared/sourceeditor/editor.js +++ b/devtools/client/shared/sourceeditor/editor.js @@ -3458,13 +3458,18 @@ class Editor extends EventEmitter { // `coordsAtPos` returns the absolute position of the line/column location // so that we have to ensure comparing with same absolute position for // CodeMirror DOM Element. + // + // Note that it may return the coordinates for a column breakpoint marker + // so it may still report as visible, if the marker is on the edge of the viewport + // and the displayed character at line/column is actually hidden after the scrollable area. const coords = cm.coordsAtPos(pos); if (!coords) { return false; } const { x, y, width, height } = cm.dom.getBoundingClientRect(); + const gutterWidth = cm.dom.querySelector(".cm-gutters").clientWidth; - inXView = withinBounds(coords.left - x, 0, width); + inXView = coords.left > x + gutterWidth && coords.right < x + width; inYView = coords.top > y && coords.bottom < y + height; } else { const { top, left } = cm.charCoords({ line, ch: column }, "local"); @@ -3548,6 +3553,7 @@ class Editor extends EventEmitter { * @param {Number} line - The line in the source * @param {Number} column - The column in the source * @param {String|null} yAlign - Optional value for position of the line after the line is scrolled. + * (Used by `scrollEditorIntoView` test helper) */ async scrollTo(line, column, yAlign) { if (this.isDestroyed()) { @@ -3566,7 +3572,7 @@ class Editor extends EventEmitter { } return cm.dispatch({ effects: EditorView.scrollIntoView(offset, { - x: "nearest", + x: "center", y: yAlign || "center", }), });