1591143 - Consider use item name as root label when Set directory root

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement, P3)

Description

[Chujun Lu]

Attachment: partial directory name.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/77.0.3865.120 Safari/537.36

Steps to reproduce:

1.Open about:home and open debugger
2.Right click data/content directory
3.Click Set directory root
4.Click the root header to remove root
5.Right click resource://activity-stream
6.Click Set directory root

Actual results:

Header text shows content;
Header text shows activity-stream.

Expected results:

I think it's more consistent to show the directory name data/content or resource://activity-stream as header text.

Comment 1

[Kyle Knaggs]

Hi! I’ve spent some time researching this issue during the past few days and if my understanding of Chujun’s intentions is correct I agree it would probably be better if we move away from using state.sources.projectDirectoryRoot when generating the text for the project root header. If the intention is for the project root header to display text that is consistent with the item that the user clicked on in the sources tree when setting the directory root then I think a simple solution could be to add a new key to the Redux store that can be used to store this data. That way we can use the already existing setProjectDirectoryRoot() function that is called whenever a user clicks "Set directory root" to update the Redux store with this information before passing it back down to PrimaryPanes/index.js. I have not worked on the sources tree before so it is possible that there may be a downside to this approach that I am unaware of. However, if there isn’t I have created a working prototype of this approach that fixes this bug and would be more than happy to submit a patch with my solution.

Comment 2

[Kyle Knaggs]

Attachment: Bug 1591143 - Use text in Sources UI to generate header after setting directory root r=davidwalsh

Before this Work In Progress patch, the text that was displayed next to the home icon after a user sets directory root was generated using state.sources.projectDirectoryRoot. This was done in an attempt to reverse engineer the text that was displayed for the corresponding item in the Sources UI. This patch removes the logic that was responsible for this. Instead, it ensures that the text next to the home icon matches the text that was displayed in the Sources UI by using the existing event handler to pass the text that is displayed in the Sources UI to the Redux store whenever a user sets directory root. The text is then passed back down to the Primary Panes UI where it is displayed as the header after a user sets directory root.

The test suites in PrimaryPanes.spec.js and setProjectDirectoryRoot.spec.js will be updated before a final patch for this issue is submitted.

Comment 3

[Chujun Lu]

Yeah, my concern was for directory names with a / in it, such as data/content. The current implementation splits the name and only renders part of it after setting the directory as root. Agree that adding the name to Redux state and using it later could be a solution.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fe257d5e4151
Use text in Sources UI to generate header after setting directory root r=davidwalsh

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/fe257d5e4151