Raw Json viewer displays unreadable address
Categories
(Firefox :: Address Bar, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr91 | --- | unaffected |
firefox99 | --- | unaffected |
firefox100 | --- | unaffected |
firefox101 | --- | wontfix |
firefox102 | --- | verified |
People
(Reporter: aflorinescu, Assigned: daisuke)
References
(Blocks 1 open bug, Regression)
Details
(Keywords: regression)
Attachments
(2 files)
Affected versions
- Nightly 101
Affected platforms
-
all , but in a different way:
* Windows : end of the json address is displayed * Ubuntu 22: end of the json address is displayed * Mac 11: end of the json address is displayed
Steps to reproduce
- Open new profile.
- Open about:telemetry
- Left bottom corner, press Raw Json.
Expected result
Raw json's address is readable
Actual result
Raw json's address is unreadable
Regression range
- Found commit message: Bug 1764411 - Cleanup toolkit/components/url-classifier/ includes
Note
Not sure what the effects outside of this scenario are.
Comment 1•2 years ago
|
||
:serge.guelton, since you are the author of the regressor, bug 1764411, could you take a look?
For more information, please visit auto_nag documentation.
Updated•2 years ago
|
Comment 2•2 years ago
|
||
Set release status flags based on info from the regressing bug 1764411
Comment 3•2 years ago
|
||
The reported regression range didn't seem plausible to me, so I took a shot at bisecting it myself and ended up at bug 1674951 as the regressor, which seems more likely? Can you please confirm that, Adrian?
Reporter | ||
Comment 4•2 years ago
•
|
||
Thanks Ryan for taking time to investigate this, bug 1674951 as regressor is correct. Updating the sumarry/description to reflect the bug that is windows/mac reproducible (the json string address is not correctly positioned), for which also my windows regression range reliably leads to Bug 1674951: Update the caret position after loading page. r=adw
However, there is a 2nd regression or behavior (which messed up the original regression range), which is mac specific and hitting it requires an intermediate step: perform an address bar search, in which case the json address on mac will look like in the bottom part of the original screenshot(blanked out). So far, didn't manage to get something probable as a regression range for that part, but the reason for which I didn't rush to log a spin'off bug is because the regression range for this part is really close to the bug 1674951 one. I'll get back to this 2nd part on Monday.
Reporter | ||
Updated•2 years ago
|
Comment 5•2 years ago
|
||
Bug 1674951 should be in the regressed by field.
Reporter | ||
Updated•2 years ago
|
Reporter | ||
Comment 6•2 years ago
|
||
(In reply to Adrian Florinescu [:aflorinescu] from comment #4)
However, there is a 2nd regression or behavior (which messed up the original regression range), which is mac specific and hitting it requires an intermediate step: perform an address bar search, in which case the json address on mac will look like in the bottom part of the original screenshot(blanked out). So far, didn't manage to get something probable as a regression range for that part, but the reason for which I didn't rush to log a spin'off bug is because the regression range for this part is really close to the bug 1674951 one. I'll get back to this 2nd part on Monday.
Redoing the regression range with the added steps for Mac, still lead me to bug 1764411. Logged the spinoff for mac separately, bug 1768401.
Assignee | ||
Comment 7•2 years ago
|
||
I also could reproduce this issue. I will fix it.
Drew, I'd like to hear your opinion.
If we set 0 to the start/end caret position explicitly, it seems that the issue is fixed at least in my env.
So, to resolve this issue, why don't we make the caret position 0 when one of the following cases?
- If the new or previous value does not indicate http[s] protocol.
- If the new value's protocol does not match the previous' protocol.
What do you think?
Comment 8•2 years ago
|
||
(In reply to Daisuke Akatsuka (:daisuke) from comment #7)
I also could reproduce this issue. I will fix it.
Thanks Daisuke.
If we set 0 to the start/end caret position explicitly, it seems that the issue is fixed at least in my env.
That makes sense because before bug 1674951 the caret index was always set to 0.
So, to resolve this issue, why don't we make the caret position 0 when one of the following cases?
- If the new or previous value does not indicate http[s] protocol.
- If the new value's protocol does not match the previous' protocol.
What do you think?
I'm not sure about that. First, we should understand why this is happening. I'm interested in knowing which code paths get hit in the STR in comment 0, and what the values of the relevant variables are. Could you investigate that please?
I did some quick debugging and found that when I click the Raw JSON link, setURI()
is called 3 times with these variable values:
isDifferentValidValue=false
isDifferentValidValue=false
-
isDifferentValidValue=true isCaretPositionEnd=true previousSelectionStart=11 previousSelectionEnd=11
I didn't log the values being set, which would have been helpful. But why is it called 3 times, why is isDifferentValidValue=true
the third time, why is the caret index 11, etc.
Assignee | ||
Comment 9•2 years ago
|
||
Thanks Drew.
I have investigated this.
When clicking “Row JSON” button, we call openJsonInFirefoxJsonViewer(). And, it seems that it calls setURI() totally 3 times inside via onLocationChange() in browser.js.
The first time, when a newly opened tab is detected as selected, it is explicitly. The target url is about:blank
. The variables of previousSelectionStart
/previousSelectionEnd
are 15 that is length of about:telemetry
.
The second time, when tabtowser.js detects location change event for about:blank
via progress listener, then calls onLocationChange
of browser.js. The target url is also about:blank
. The variables of previousSelectionStart
/previousSelectionEnd
are 11 that is length of about:blank
.
The third time, when tabtowser.js detects location change event for the data URL of JSON via progress listener at the same place of above. The target url is the data url. The variables of previousSelectionStart
/previousSelectionEnd
are 11 that is length of about:blank
. And, in this case, as previousSelectionEnd
is as same as the length of about:blank
, isCaretPositionEnd
will be true, and the caret comes to move to end. It seems this is a cause.
The solution I am thinking now is:
- When opening a new tab or restoring a tab(= !valid), make the caret position 0.
- Furthermore, in the current algorithm, if the string length is 0 and previousSelectionEnd is also 0, isCaretPositionEnd will be true, the caret comes to move to end. I think we should keep the caret position 0 in this case.
What do you think?
I will try to write a patch with the way once and will check whether there are test failures by the solution.
Assignee | ||
Comment 10•2 years ago
|
||
Assignee | ||
Comment 11•2 years ago
|
||
It seems that only browser_retainedResultsOnFocus.js
test fails.
Comment 12•2 years ago
|
||
That sounds good to me. Have you been able to figure out the test failure? Thanks for investigating and sorry for the delay.
Assignee | ||
Comment 13•2 years ago
|
||
Updated•2 years ago
|
Updated•2 years ago
|
Comment 15•2 years ago
|
||
Pushed by dakatsuka.birchill@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fbd7e82e204f Change the logic for taking over the caret position. r=adw
Comment 16•2 years ago
|
||
bugherder |
Updated•2 years ago
|
Comment 17•2 years ago
|
||
Verified as fixed using Firefox 102 beta 5 under Win 10 64-bit, Mac OSX 12.3 and Ubuntu 22.04. Filled bug 1773333 as a follow-up for OSX only.
Description
•