Closed Bug 1188667 Opened 5 years ago Closed 5 years ago

[meta] View Source for 2.5

Categories

(Firefox OS Graveyard :: Gaia::System, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(feature-b2g:2.5+, b2g-master fixed)

RESOLVED FIXED
FxOS-S6 (04Sep)
feature-b2g 2.5+
Tracking Status
b2g-master --- fixed

People

(Reporter: nyee, Unassigned)

References

Details

(Keywords: feature, meta)

User Story

"I want to be able to view source of apps"

Attachments

(2 files, 11 obsolete files)

No description provided.
Depends on: 1188670
Depends on: 1188671
Depends on: 1188673
Assignee: nobody → drs
Attachment #8648214 - Flags: review?(timdream)
Comment on attachment 8648214 [details] [review]
[gaia] DouglasSherk:1188667-view-source > mozilla-b2g:master

How do you test this?
Flags: needinfo?(drs)
I mean, automation tests, of course.
That's a good question. I'm not sure how we can test add-ons. Fabrice, any suggestions?
Flags: needinfo?(drs) → needinfo?(fabrice)
Are the dependent bugs on here hard requirements for 2.5? (because they have the 2.5+ feature flag marked and they haven't been resolved)
Flags: needinfo?(drs)
We talked a bit on irc. On gecko we have mochitests to check that we inject scripts properly, but it's a bit unclear how we can test that in gaia integration tests.

Maybe something like:
frameA.html (the 'driver') includes frameB.html, we trigger view-source on frameB.html, and frameA reaches into frameB to check that the content of the view-source iframe is as expected.
Flags: needinfo?(fabrice)
Flags: needinfo?(nhirata.bugzilla)
A Gij test accompany with the patch would be good.
Attachment #8648214 - Attachment is obsolete: true
Flags: needinfo?(drs)
Attachment #8648214 - Flags: review?(timdream)
feature-b2g: --- → 2.5+
Component: Gaia → Gaia::System
Assignee: drs → nobody
Comment on attachment 8653191 [details] [review]
[gaia] DouglasSherk:1188667-view-source > mozilla-b2g:master

We're no longer using this, instead opting to build the feature into the core OS.
Attachment #8653191 - Attachment is obsolete: true
FYI, No-Jun might be the tester going forward for this feature.
Flags: needinfo?(nhirata.bugzilla)
Attachment #8653761 - Attachment description: View Source Window Screenshot → View Source window screenshot
Attachment #8653764 - Attachment description: View source window screenshot 3 → View Source window screenshot 3
Attached file View Source demo video (obsolete) —
Please see the other attachments in this bug for screenshots.

Note that we don't have much control over the text styling, colors, etc. as they are all shared with Firefox and come from the 'view-source:' URL scheme. I can, however, change the look of the window, transparency, method to open/close it, etc.
Attachment #8653766 - Flags: ui-review?(jsavory)
Attachment #8653766 - Flags: ui-review?(amlee)
Comment on attachment 8653766 [details]
View Source demo video

Hey Doug, overall it looks good, just a couple things:

- I'm worried how you close the screen isn't obvious enough right now, so instead we could add a header to the screen, titled "View Source" with an 'x' on the left side. Tapping the 'x' would simply close the screen. 

- Its a bit hard to tell in the video, but we could make the screen full screen (if its not already) if we add the header. I think overall it'll look much cleaner that way. 

Let me know if this makes sense or if you have any other questions.
Attachment #8653766 - Flags: ui-review?(jsavory) → ui-review-
Comment on attachment 8653766 [details]
View Source demo video

Just to add to Jacqueline's feedback. I would make the header white with the grey used in settings for the header text and "x" button.
Attachment #8653766 - Flags: ui-review?(amlee) → ui-review-
Attached image View_Source.png (obsolete) —
Mock for reference to feedback in comment 18.
Attached image View Source screenshot (obsolete) —
Thanks for the reviews. I've addressed the issues that you both raised. You can try it out by running attachment 8653198 [details] [review] locally.

Note that I made two changes that deviate from the spec provided in attachment  8655524 [details]:

1. I removed the transparency because I was finding that it was getting in the way of reading.
2. The close button ("X") is on the left instead of the right. This is a convention that we've been following for other apps. It's not trivial to change this as the positioning is built into <gaia-header>, so I'd have to patch that.
Attachment #8653761 - Attachment is obsolete: true
Attachment #8653762 - Attachment is obsolete: true
Attachment #8653763 - Attachment is obsolete: true
Attachment #8653764 - Attachment is obsolete: true
Attachment #8653765 - Attachment is obsolete: true
Attachment #8653766 - Attachment is obsolete: true
Attachment #8655905 - Flags: ui-review?(jsavory)
Attachment #8655905 - Flags: ui-review?(amlee)
Comment on attachment 8655905 [details]
View Source screenshot

Hi, 

Can you change the header to white? It looks a bit strange that we're using the Settings header with a white background for the code portion. 

Also please change View source to View Source (capitalized S) to follow our header convention. 

Thanks!
Attachment #8655905 - Flags: ui-review?(amlee) → ui-review-
Attached image View_Source.png (obsolete) —
Reference mock
Attachment #8655524 - Attachment is obsolete: true
Attached image View_Source.png
Attachment #8656672 - Attachment is obsolete: true
Attached image View Source screenshot
Fixed based on feedback in comment 21. As before, you can try this feature out by building and flashing attachment 8653198 [details] [review] locally.
Attachment #8655905 - Attachment is obsolete: true
Attachment #8655905 - Flags: ui-review?(jsavory)
Attachment #8656708 - Flags: ui-review?(jsavory)
Attachment #8656708 - Flags: ui-review?(amlee)
Comment on attachment 8656708 [details]
View Source screenshot

Hi Doug, 

Looks good to me. Thanks!
Attachment #8656708 - Flags: ui-review?(amlee) → ui-review+
Comment on attachment 8656708 [details]
View Source screenshot

Looks good to me as well!
Attachment #8656708 - Flags: ui-review?(jsavory) → ui-review+
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: FxOS-S5 (21Aug) → FxOS-S6 (04Sep)
Depends on: 1204014
Depends on: 1204004
Depends on: 1204011
Depends on: 1204002
Depends on: 1204001
Depends on: 1204000
No longer depends on: 1204004
No longer depends on: 1204000
Depends on: 1246766
You need to log in before you can comment on or make changes to this bug.