Closed Bug 1198569 Opened 8 years ago Closed 8 years ago
Multi-line comments wrap in inspector view
Text nodes have newlines replaced with spaces so that they only take up one line in the inspector. Comments should too.
Whiteboard: [good first bug][lang=js][lang=css][polish-backlog]
I would like to work on this. Where is the relevant code located? I tried browser/devtools/inspector/breadcrumbs.js but didn't find anything.
The code that generates the DOM tree preview in the inspector is browser/devtools/markupview/markup-view.js There are various MarkupContainer implementations in this file, one for each type of node shown in the inspector, and each of them create the necessary markup using some HTML template in browser/devtools/markupview/markup-view.xhtml. For comments, see the <span id="template-comment"> in that file. This is the piece of HTML that is used to generate comment nodes in the inspector. I hope that's enough to dig around by yourself a little more. If not, let me know, I can give more hints about how to solve this bug.
Comment on attachment 8654818 [details] [diff] [review] fix.patch Review of attachment 8654818 [details] [diff] [review]: ----------------------------------------------------------------- This essentially makes comment nodes look like text nodes. I think that's good. Can you push to TRY to make sure no tests fail because of this (I highly doubt we have a test that checks this kind things, but you never know).
Attachment #8654818 - Flags: review?(pbrosset) → review+
I have filed a request for level 1 commit access here: https://bugzilla.mozilla.org/show_bug.cgi?id=1200723 Would you be able to vouch for me ?
(In reply to Rohan Rawat from comment #5) > I have filed a request for level 1 commit access here: > > https://bugzilla.mozilla.org/show_bug.cgi?id=1200723 > > Would you be able to vouch for me ? Don't worry I'll push to try for you for now. Let's keep this request bug open and I'll vouch for you after we've worked a little bit more together. Are you interested in working on other bugs? Feel free to take a look at http://firefox-dev.tools , it should help you find good bugs to work on (bugs that are either mentored or marked as easy). Also, feel free to join our #devtools IRC channel to ask for bugs. Try build URL: https://treeherder.mozilla.org/#/jobs?repo=try&revision=83e5c0c3a9db
Assignee: nobody → rohan1395
Status: NEW → ASSIGNED
Ah one more thing, the patch isn't formatted correctly I think. It's just a diff, it has no commit message nor an author. This should help: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch and this: https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Comment on attachment 8655965 [details] [diff] [review] Added the 'white-space: normal' rule to the comments template Review of attachment 8655965 [details] [diff] [review]: ----------------------------------------------------------------- This looks good. Thanks for creating this new patch. Just one last thing needed: can you add the suffix ", r=<reviewer-username>" to the commit message? So: Bug 1198569 - Added the 'white-space: normal' rule to the comments template, r=pbro Once done, you don't need to re-ask for review, you can simply upload your new patch, mark the old one as obsolete, and then add the keyword "checkin-needed" at the top of the bug. The try build isn't done, but the tests are green on one platform already, and knowing the type of change you made, I'm not expecting anything to go badly. So just set the flag after you've uploaded the new patch, that will put the bug on the sheriffs' radar and they will check it in the repository.
Attachment #8655965 - Flags: review?(pbrosset) → review+
You might have attached the wrong patch, the new one is identical to the previous one. Also you forgot to set the review flag to R+ when attaching it (in the drop-down where you can choose R? to ask someone to review, you can also choose R+).
Sorry about that. I am still trying to figure out bzexport.
You need to log in before you can comment on or make changes to this bug.