Closed
Bug 1198569
Opened 9 years ago
Closed 9 years ago
Multi-line comments wrap in inspector view
Categories
(DevTools :: Inspector, defect)
DevTools
Inspector
Tracking
(firefox43 fixed)
RESOLVED
FIXED
Firefox 43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: darktrojan, Assigned: rohan1395)
Details
(Whiteboard: [good first bug][lang=js][lang=css][polish-backlog])
Attachments
(2 files, 4 obsolete files)
3.43 KB,
image/png
|
Details | |
1.19 KB,
patch
|
rohan1395
:
review+
|
Details | Diff | Splinter Review |
Text nodes have newlines replaced with spaces so that they only take up one line in the inspector. Comments should too.
Updated•9 years ago
|
Whiteboard: [good first bug][lang=js][lang=css][polish-backlog]
Assignee | ||
Comment 1•9 years ago
|
||
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.
Flags: needinfo?(geoff)
Comment 2•9 years ago
|
||
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.
Flags: needinfo?(geoff)
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8654818 -
Flags: review?(pbrosset)
Comment 4•9 years ago
|
||
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+
Assignee | ||
Comment 5•9 years ago
|
||
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 ?
Flags: needinfo?(pbrosset)
Comment 6•9 years ago
|
||
(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
Flags: needinfo?(pbrosset)
Comment 7•9 years ago
|
||
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
Assignee | ||
Comment 8•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8655965 -
Flags: review?(pbrosset)
Comment 9•9 years ago
|
||
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+
Assignee | ||
Comment 10•9 years ago
|
||
Attachment #8655977 -
Flags: review?(pbrosset)
Assignee | ||
Updated•9 years ago
|
Attachment #8655965 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Attachment #8655977 -
Flags: review?(pbrosset)
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 11•9 years ago
|
||
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+).
Keywords: checkin-needed
Updated•9 years ago
|
Attachment #8654818 -
Attachment is obsolete: true
Assignee | ||
Comment 12•9 years ago
|
||
Sorry about that. I am still trying to figure out bzexport.
Assignee | ||
Updated•9 years ago
|
Attachment #8655977 -
Attachment is obsolete: true
Assignee | ||
Comment 13•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8656020 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Attachment #8656021 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 14•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/bcda316bf139
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•