Closed
Bug 1004679
Opened 10 years ago
Closed 10 years ago
Use text-align: start or end instead of left or right in DevTools themes
Categories
(DevTools :: General, defect)
DevTools
General
Tracking
(firefox34 verified)
VERIFIED
FIXED
Firefox 34
Tracking | Status | |
---|---|---|
firefox34 | --- | verified |
People
(Reporter: bgrins, Assigned: alexandre-cote, Mentored)
Details
(Whiteboard: [bugday-20140820])
Attachments
(1 file, 2 obsolete files)
5.93 KB,
patch
|
bgrins
:
review+
|
Details | Diff | Splinter Review |
There was a patch posted in Bug 552486, which was intended for doing this in SeaMonkey, but actually does it for DevTools. Patch is here: https://bugzilla.mozilla.org/attachment.cgi?id=8415992&action=edit
Reporter | ||
Comment 1•10 years ago
|
||
Alex, would you mind uploading your patch from Bug 552486 here? One thing to add about the original patch is that the three separate ruleview.css files have been merged into one at browser/themes/shared/devtools/ruleview.css.
Flags: needinfo?(alexandre-cote)
Reporter | ||
Comment 2•10 years ago
|
||
A search for text align right (a few of these are just for tests and don't matter, and the codemirror.css one should be ignored). http://dxr.mozilla.org/mozilla-central/search?q=+path%3A*devtools*+%22text-align%3A+right%22&redirect=true
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8416967 -
Flags: review?(bgrinstead)
Flags: needinfo?(alexandre-cote)
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #1) > Alex, would you mind uploading your patch from Bug 552486 here? One thing > to add about the original patch is that the three separate ruleview.css > files have been merged into one at > browser/themes/shared/devtools/ruleview.css. I have done it.
Reporter | ||
Updated•10 years ago
|
Whiteboard: [good first bug][mentor=bgrins][lang=css]
Reporter | ||
Comment 5•10 years ago
|
||
Comment on attachment 8416967 [details] [diff] [review] bug552486_text-align_startorend.diff Review of attachment 8416967 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for adding the patch. A couple of notes: 1) There is an error applying this patch on the file: browser/themes/shared/devtools/app-manager/device.css. Pull down the latest version of the code and update the patch based on this. 2) Please edit your commit message to say: "Bug 1004679 - Use text-align: start or end instead of left or right in DevTools themes; r=bgrins".
Attachment #8416967 -
Flags: review?(bgrinstead)
Reporter | ||
Updated•10 years ago
|
Assignee: nobody → alexandre-cote
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8416967 -
Attachment is obsolete: true
Attachment #8420673 -
Flags: review?(bgrinstead)
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Alex from comment #6) > Created attachment 8420673 [details] [diff] [review] > second try Could it be possible to have a review soon ?
Reporter | ||
Comment 8•10 years ago
|
||
(In reply to Alex from comment #7) > (In reply to Alex from comment #6) > > Created attachment 8420673 [details] [diff] [review] > > second try > > Could it be possible to have a review soon ? I will take a look at this today. It may take a little bit to do the review, since I will need to find each change individually and make sure RTL support wasn't relying on the old behavior
Reporter | ||
Comment 9•10 years ago
|
||
Comment on attachment 8420673 [details] [diff] [review] second try Review of attachment 8420673 [details] [diff] [review]: ----------------------------------------------------------------- Ryan, there are a couple of changes to app manager CSS here so I wanted to check with you to see if the changes to device.css looked OK.
Attachment #8420673 -
Flags: feedback?(jryans)
Comment on attachment 8420673 [details] [diff] [review] second try Review of attachment 8420673 [details] [diff] [review]: ----------------------------------------------------------------- Well, the App Manager UI doesn't try very hard to be RTL compatible today... but anyway, the changes made here seem identical to the current state for both LTR and RTL. So, seems fine!
Attachment #8420673 -
Flags: feedback?(jryans) → feedback+
Reporter | ||
Comment 11•10 years ago
|
||
Comment on attachment 8420673 [details] [diff] [review] second try Review of attachment 8420673 [details] [diff] [review]: ----------------------------------------------------------------- OK, this all looks good - but can you update the patch one more time after pulling changes please? There is an error applying the patch on the latest code: 1 out of 1 hunks FAILED -- saving rejects to file browser/themes/shared/devtools/common.css.rej
Attachment #8420673 -
Flags: review?(bgrinstead)
Updated•10 years ago
|
Mentor: bgrinstead
Whiteboard: [good first bug][mentor=bgrins][lang=css] → [good first bug][lang=css]
Comment 12•10 years ago
|
||
I am new to bugzilla help me to find how to get assigned to bug and how to resolve it.
Reporter | ||
Comment 13•10 years ago
|
||
(In reply to s.mahavarkar from comment #12) > I am new to bugzilla help me to find how to get assigned to bug and how to > resolve it. Actually, the attached patch is pretty much ready to go, just needs some rebasing. Please take a look at the bugs marked as good first bugs list: http://mzl.la/1AcYOKF or the list of all mentored bugs: http://mzl.la/1pqY2q8 for something to get started on.
Reporter | ||
Comment 14•10 years ago
|
||
Rebased and pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=02b42a050f7a
Attachment #8420673 -
Attachment is obsolete: true
Attachment #8472304 -
Flags: review+
Reporter | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 15•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/313e2d2e3df0
Keywords: checkin-needed
Whiteboard: [good first bug][lang=css] → [good first bug][lang=css][fixed-in-fx-team]
Comment 16•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/313e2d2e3df0
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][lang=css][fixed-in-fx-team] → [good first bug][lang=css]
Target Milestone: --- → Firefox 34
Updated•10 years ago
|
QA Whiteboard: [good first verify]
Updated•10 years ago
|
Flags: qe-verify+
Comment 17•10 years ago
|
||
Tried applying text-align: start and text-align: end on different sites and different tags, seems to work fine.
Status: RESOLVED → VERIFIED
QA Whiteboard: [good first verify] → [bugday-20140820]
status-firefox34:
--- → verified
Whiteboard: [good first bug][lang=css] → [bugday-20140820]
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•