1127417 - Replace hardcoded "…" with internationalized version in markup view

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Brian Grinstead [:bgrins]]

See "XXX: internationalize the elliding" at https://dxr.mozilla.org/mozilla-central/source/browser/devtools/markupview/markup-view.js#2075.

We just need to replace this character with:

Services.prefs.getComplexValue("intl.ellipsis", Ci.nsIPrefLocalizedString).data

Comment 1

[vaibhav [:vaibhavChoudhary]]

:Brain i would really like to work on this bug.I m new to open source and i think this can help me to further venture in open source world.

Comment 2

[Brian Grinstead [:bgrins]]

(In reply to vaibhav from comment #1)
> :Brain i would really like to work on this bug.I m new to open source and i
> think this can help me to further venture in open source world.

Hello, to get started first take a look at https://wiki.mozilla.org/DevTools/GetInvolved for some information and resources about working on DevTools.

Once you are ready to start pulling down the code, follow the instructions at https://wiki.mozilla.org/DevTools/Hacking#First_Build.  Feel free to hop onto the #devtools IRC channel and ask if you have any issues getting started up.

Comment 3

[Brian Grinstead [:bgrins]]

Once you get a build running, the patch itself should be pretty simple.  The hard part is going to be getting your development environment set up and then learning how to build and submit a patch: https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F.


We will just need a line like this near the top of the file with any other consts:

const ELLIPSIS = Services.prefs.getComplexValue("intl.ellipsis", Ci.nsIPrefLocalizedString).data;

And then just replace the hardcoded "…" with ELLIPSIS.

Comment 4

[vaibhav [:vaibhavChoudhary]]

Attachment: bug1127417_InternationalizedMarkup.diff

Comment 5

[vaibhav [:vaibhavChoudhary]]

Brian ,would greatly appriciate if u can review the patch and let me know if there are any problems with it.

Comment 6

[vaibhav [:vaibhavChoudhary]]

Brain ,do i have to do anything else regarding this patch ?

Comment 7

[Brian Grinstead [:bgrins]]

Comment on attachment 8557262 [details] [diff] [review]
bug1127417_InternationalizedMarkup.diff

Review of attachment 8557262 [details] [diff] [review]:
-----------------------------------------------------------------

This isn't quite right (see inline comments for what needs to change).  You can test your changes on a page like: http://jsbin.com/yubaniseze/1.  Inspect the div with the really long text content and expand it in the markup view - you should see the ellipses in this case.

::: browser/devtools/markupview/markup-view.js
@@ +2072,5 @@
>  
>    update: function() {
>      if (!this.selected || !this.node.incompleteValue) {
>        let text = this.node.shortValue;
> +      Services.prefs.getComplexValue("intl.ellipsis", Ci.nsIPrefLocalizedString).data;

Remove this line altogether

@@ +2077,2 @@
>        if (this.node.incompleteValue) {
>          text += "…";

You need to replace the "…" here with ELLIPSIS, and just remove the line with the comment.

Comment 8

[vaibhav [:vaibhavChoudhary]]

sorry for check-in ...will changing code right now ..

Comment 9

[vaibhav [:vaibhavChoudhary]]

brian ,can you please explain what do you mean by "nspect the div with the really long text content and expand it in the markup view".

Comment 10

[Brian Grinstead [:bgrins]]

(In reply to vaibhav [:vaibhavChoudhary] from comment #9)
> brian ,can you please explain what do you mean by "nspect the div with the
> really long text content and expand it in the markup view".

Open this in a tab: http://jsbin.com/yubaniseze/1.  Then right click on the text and say Inspect Element.  Notice that a <div> gets highlighted in the markup view.  Now expand this div in devtools by clicking on the triangle to the left of it.  See the "..." at the end of it and how it cuts off the text of the element?  This is the string that we want to update.

So you can test that you are changing the right thing by modifying that string to something else and see if it is reflected in your build.  Once you know you are changing the right thing, just swap it out with ELLIPSIS.

Comment 11

[vaibhav [:vaibhavChoudhary]]

Brian,Thanx

Comment 12

[vaibhav [:vaibhavChoudhary]]

Attachment: rev1 : modified and tested

Comment 13

[Brian Grinstead [:bgrins]]

Comment on attachment 8557308 [details] [diff] [review]
rev1 : modified and tested

Review of attachment 8557308 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/markupview/markup-view.js
@@ +2076,2 @@
>        if (this.node.incompleteValue) {
> +        text = ELLIPSIS;

This should remain += so we don't end up overwriting the actual text

Comment 14

[vaibhav [:vaibhavChoudhary]]

Attachment: rev_2 += added

brian ,i added the "+=" sign there but if you notice problem still persists. check the rev1 after changing "+=" to "=" whole text appear as ".." and on click it expands.but with rev2 which is this current patch the small section of text appears with ".." .I would really appriciate if u can explain this to me.

Comment 15

[Brian Grinstead [:bgrins]]

(In reply to vaibhav [:vaibhavChoudhary] from comment #14)
> after changing "+=" to "=" whole text appear as ".." and on
> click it expands.but with rev2 which is this current patch the small section
> of text appears with ".." .I would really appriciate if u can explain this
> to me.

This is the expected behavior :).  The patch looks good, pushed it to the test servers: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fb42294b4353

Comment 16

[Brian Grinstead [:bgrins]]

Comment on attachment 8557462 [details] [diff] [review]
rev_2  += added

Review of attachment 8557462 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me

Comment 17

[Brian Grinstead [:bgrins]]

Pushed to fxteam: https://hg.mozilla.org/integration/fx-team/rev/2098158a0374

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2098158a0374