Bug 807881 - [Bluetooth] Need to check SDCard status before receiving file by other devices #6247
Bug 807881 - [Bluetooth] Need to check SDCard status before receiving file by other devices #6247
Conversation
/botio lint |
From: Bot.io (Main)ReceivedCommand cmd_lint from @ian-liu received. Current queue size: 2 Live output at: http://50.116.11.35:8000/05995e59de6e0bd/output.txt |
From: Bot.io (Main)FailedFull output at http://50.116.11.35:8000/05995e59de6e0bd/output.txt Total script time: 3.59 mins Lint: FAILED |
openReceivedFile: function bt_openReceivedFile(evt) { | ||
// Launch the gallery with an open activity to view this specific photo | ||
var fileName = evt.fileName; | ||
var filePath = '/sdcard/Downloads/' + fileName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's "/downloads/bluetooth", at least in my test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And if this path is defined in gecko, it might be better to add some comments about that it's a predefined path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know where the file gets stored by the bluetooth module, but device storage uses /sdcard as its root, so you don't include that in the path. I'm hoping that gecko uses a capital D and strores files in Downloads/ instead of downloads/. But that doesn't really matter.
One final comment.. In the init() method of bluetooth_transfer.js, you have unnecessary nested functions for mozSetMessageHandler(). That would be a good place to just pass bound copies of your methods. |
"bluetooth":{}, | ||
"mozBluetooth":{}, | ||
"device-storage:pictures":{}, | ||
"device-storage:music":{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comma is missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, just forget it, my mistake.
/botio lint |
From: Bot.io (Main)ReceivedCommand cmd_lint from @ian-liu received. Current queue size: 0 Live output at: http://50.116.11.35:8000/679c859a52805f5/output.txt |
From: Bot.io (Main)FailedFull output at http://50.116.11.35:8000/679c859a52805f5/output.txt Total script time: 1.82 mins Lint: FAILED |
Refined the pull request according the above comment. Note: |
@davidflanagan and @dominickuo |
@ian-liu Let me help you first, let's discuss offline. |
@davidflanagan and @dominickuo |
function bt_gotReceivingFileConfirmationMessage(message) { | ||
self.onReceivingFileConfirmation(message); | ||
} | ||
this.bluetoothMessageHandler.bind(this, 'receiving-file-confirmation') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't this just be
this.onReceivingFileConfirmation.bind(this)
CustomDialog.show(_('cannotOpenFile'), body, confirm); | ||
}, | ||
|
||
confirmUnknowMediaPrompt: function bt_confirmUnknowMediaPrompt() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is such a simple function that I'd just nest it in the method above. It doesn't use this, and doesn't need to be a method.
/botio lint |
According @davidflanagan and @dominickuo 's comment, I have refined the patch. |
// use the file.type to replace the empty fileType which is given by API | ||
var fileType = ''; | ||
var fileName = file.name; | ||
var filetype = (fileType != '') ? fileType : file.type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is filetype typo? looks like fileType, not filetype
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here (fileType != '') will always be false. isn't it?
… file by other devices r=dkuo
@ian-liu Looks good to me, r+, and I will send another patch for Gallery to sync with your patch, which caused by Bug 811615. |
@dominickuo Thanks for your reviewed effort. We can merge the pr now. |
Bug 807881 - [Bluetooth] Need to check SDCard status before receiving file by other devices
No description provided.